New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
net/http: make DNS lookup timeouts return net.DNSError instead of poll.TimeoutError #39178
Comments
This proposal has been added to the active column of the proposals project |
/cc @neild and @ianlancetaylor for thoughts |
It looks like if we change the error on that code path to be a net.DNSError, then we would lose what Go 1.15 established, that you can use errors.Is(err, os.ErrDeadlineExceeded) to check for the exceeded deadline. That seems like a mistake. |
errors.Is(err, os.ErrDeadlineExceeded) currently returns false for net.timeoutError despite a deadline being exceeded. I'm not sure if any capability would be lost when DNS times out. |
@mhale You're right. What we did in 1.15 was establish that Making the proposed change seems reasonable to me. |
Making this a |
If |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
Change https://golang.org/cl/353400 mentions this issue: |
I get DNS lookup timeouts many times a day when running Go HTTP clients on Kubernetes, where UDP packets are sometimes dropped due to a race condition in the Linux kernel. I'd like to propose a small standard library change to help debug these scenarios.
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes. This occurs in Go 1.14.[1-3] and
version devel +c88f6989e1 Tue May 19 04:10:43 2020 +0000 linux/amd64
.What operating system and processor architecture are you using (
go env
)?It seems to apply to all platforms. I'm experiencing it most often on Kubernetes on Linux but the executable is compiled on a regular Linux server.
go env
OutputWhat did you do?
For any http.Client use with a timeout set on the transport, when the timeout is exceeded and an error is returned it can be challenging to discover the cause of the timeout from the error message. Is it DNS related or TCP related or something else? Consider the following simulation of a DNS lookup that does not return within the timeout.
DNS lookup timeout simulator code
In the event of a DNS lookup timeout, the output is:
In the event of a TCP connection timeout (comment out the
time.Sleep()
call) the output is:The missing IP address in the DNS lookup timeout output is the only clue that the root cause is a DNS problem, rather than a TCP problem. If users do not expect to see an IP address in that error message, they won't know it is missing, and their debugging and testing effort may be wasted investigating possible TCP-related causes, instead of investigating DNS (which normally runs over UDP).
I think the error message could communicate the root cause to users more clearly.
What did you expect to see?
What did you see instead?
Proposed solution
After a DNS lookup timeout, the error returned by
client.Get()
looks like this in Go 1.14.3:Note: The &poll.TimeoutError{} will become &net.timeoutError{} in future releases after commit
d422f54.
The resolver in net/lookup.go is the source of the
*poll.TimeoutError
. Instead it could return a*net.DNSError
error when a lookup times out. Doing so would provide additional error context for all use cases (not only http.Client) and may save debugging time. For exampleclient.Get()
could return:This would preserve the existing "i/o timeout" string and the ability to call
err.Timeout()
, while adding "lookup example.com: " to the output and the capability to explicitly check for a*net.DNSError
. The output from my simulator would also be what I'm expecting to see:The following change to implement this works in my usage and passes the tests in
all.bash
.git diff
OutputI can submit a pull request if anyone thinks that this is a good idea and implementation.
The text was updated successfully, but these errors were encountered: