Skip to content
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: Dial should not conceal effective errors #18183

Open
mikioh opened this issue Dec 3, 2016 · 1 comment
Open

net: Dial should not conceal effective errors #18183

mikioh opened this issue Dec 3, 2016 · 1 comment
Milestone

Comments

@mikioh
Copy link
Contributor

mikioh commented Dec 3, 2016

https://go-review.googlesource.com/8768 changed to return the first detected error on consecutive or parallel dialing as opposed to the previous. Unfortunately this change makes debugging hard. Also it probably makes package developers using the net package confusing.

For example, assuming we need to fetch some resource from golang.org on the node which has IP dual stack functionality and IPv4-only connectivity to golang.org, the current implementation returns an error regarding IPv6 transport even when the IPv4 connectivity is lost in the middle of the path.

I guess that there are two options; a) returning all detected errors by either a nested OpError or new error type in OpError when the connection setup function fails, b) returning the most effective error by referring to information on IP routing and forwarding.

I'd prefer the former. @pmarks-net, What do you think?

@mikioh mikioh added this to the Go1.9 milestone Dec 3, 2016
@pmarks-net
Copy link
Contributor

If your only choices are "return the first error" and "return the last error", then the first is usually better. Say that a user has IPv4-only Internet access, and encounters a period of packet loss. Dial tries IPv4 (which times out) followed by IPv6 (network unreachable). Displaying the last error leads to "IPv6 broke my Internet connection! How do I disable IPv6?"

I agree that returning all errors is the ideal solution, but at some point that list still needs to be reduced to a single human-readable error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants