[GH-ISSUE #1176] ResolverOpts::attempts is actually number of re-try attempts #621

New issue

Closed

opened 2026-03-15 23:30:12 +03:00 by kerem · 5 comments

kerem commented

2026-03-15 23:30:12 +03:00

Owner

Originally created by @lukaspustina on GitHub (Jul 28, 2020).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1176

Describe the bug
I'm not sure if this is really a bug, but IMHO the documentation of ResolverOpts suggests a different meaning of the field attempts:

Documentation says:

attempts: usize - Number of attempts before giving up. Defaults to 2

The code seems to indicate otherwise though. The number of attempts is decremented after the first try; so the field represents the number of re-try attempts -- retry_dns_handle.rs:74

  fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
      // loop over the future, on errors, spawn a new future
      //  on ready and not ready return.
      loop {
          match self.future.poll_unpin(cx) {
              Poll::Ready(Err(e)) => {
                  if self.remaining_attempts == 0 {
                      return Poll::Ready(Err(e));
                  }

                  self.remaining_attempts -= 1;
                  // TODO: if the "sent" Message is part of the error result,
                  //  then we can just reuse it... and no clone necessary
                  let request = self.request.clone();
                  self.future = self.handle.send(request);
              }
              poll => return poll,
          }
      }

To Reproduce
Count the amount of UDP packets sent with Wireshark or tcpdump. But it's actually in the code.

Expected behavior
From the documentation I expected to see up to ResolverOpts::attempts packets.

Version:
Crate: resolver
Version: 0.19.5

Originally created by @lukaspustina on GitHub (Jul 28, 2020). Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/1176 **Describe the bug** I'm not sure if this is really a bug, but IMHO the documentation of `ResolverOpts` suggests a different meaning of the field `attempts`: * Documentation says: > attempts: usize - Number of attempts before giving up. Defaults to 2 * The code seems to indicate otherwise though. The number of attempts is decremented _after_ the first try; so the field represents the number of re-try attempts -- `retry_dns_handle.rs:74` ``` fn poll(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> { // loop over the future, on errors, spawn a new future // on ready and not ready return. loop { match self.future.poll_unpin(cx) { Poll::Ready(Err(e)) => { if self.remaining_attempts == 0 { return Poll::Ready(Err(e)); } self.remaining_attempts -= 1; // TODO: if the "sent" Message is part of the error result, // then we can just reuse it... and no clone necessary let request = self.request.clone(); self.future = self.handle.send(request); } poll => return poll, } } ``` **To Reproduce** Count the amount of UDP packets sent with Wireshark or tcpdump. But it's actually in the code. **Expected behavior** From the documentation I expected to see up to `ResolverOpts::attempts` packets. **Version:** Crate: resolver Version: 0.19.5

kerem closed this issue

2026-03-15 23:30:17 +03:00

kerem commented

2026-03-15 23:30:23 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

Interesting, seems like a poor interpretation of what that value means! Yes, I would expect that is being more used as a retry counter rather than more strictly as the total number of attempts, including the first request.

I suppose we can either fix the code to use this and include the first attempts as well?

@bluejekyll commented on GitHub (Aug 4, 2020): Interesting, seems like a poor interpretation of what that value means! Yes, I would expect that is being more used as a retry counter rather than more strictly as the total number of attempts, including the first request. I suppose we can either fix the code to use this and include the first attempts as well?

kerem commented

2026-03-15 23:30:28 +03:00

Author

Owner

@lukaspustina commented on GitHub (Aug 4, 2020):

I don't mind either fix. I guess, it's easier and friendlier to current users of the crate to just clarify the documentation since a re-interpretation in the code could possibly brake existing usage of the parameter.

@lukaspustina commented on GitHub (Aug 4, 2020): I don't mind either fix. I guess, it's easier and friendlier to current users of the crate to just clarify the documentation since a re-interpretation in the code could possibly brake existing usage of the parameter.

kerem commented

2026-03-15 23:30:33 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 4, 2020):

agreed. If you'd like to submit a PR with the documentation updated, that would be really helpful!

@bluejekyll commented on GitHub (Aug 4, 2020): agreed. If you'd like to submit a PR with the documentation updated, that would be really helpful!

kerem commented

2026-03-15 23:30:38 +03:00

Author

Owner

@lukaspustina commented on GitHub (Aug 5, 2020):

@bluejekyll: Done.

@lukaspustina commented on GitHub (Aug 5, 2020): @bluejekyll: Done.

kerem commented

2026-03-15 23:30:43 +03:00

Author

Owner

@bluejekyll commented on GitHub (Aug 7, 2020):