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/http: Transport.DialContext breaks existing code #16703

Closed
rogpeppe opened this issue Aug 15, 2016 · 3 comments
Closed

net/http: Transport.DialContext breaks existing code #16703

rogpeppe opened this issue Aug 15, 2016 · 3 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Aug 15, 2016

In Go 1.7 the new http.Transport.DialContext field overrides the Dial field.
This breaks any existing code that takes the http.DefaultTransport,
sets Dial and expects network calls to be made using that value.
This did actually break some of our test code that used this functionality
to prevent accidental external network access in tests.

For better backward compatibility, perhaps it might be better to make Dial
override DialContext - existing code will continue to work unchanged
and most new code will never touch Dial, so the override isn't so important
in that case.

@bradfitz
Copy link
Contributor

This might've been something to consider changing during the betas or first 5 release candidates. Too late now, though.

There might be something we could add to the Go 1 compatibility document (similar to what we added about unkeyed struct literals) about promises (or lack thereof?) here.

@bradfitz bradfitz changed the title net/http: http.DialContext breaks existing code net/http: Transport.DialContext breaks existing code Aug 15, 2016
@rogpeppe
Copy link
Contributor Author

Sorry for the late report - I only encountered the issue this morning. I can accept that it won't be changed now, but I think perhaps the issue points towards a better way to do compatibility in general. I would certainly have been inclined to give precedence to new fields until I saw this.

@rakyll rakyll added this to the Go1.8 milestone Aug 15, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 7, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Nov 1, 2016

I went to document this, but I'm not sure what to document.

Closing for now, but please reopen if you have suggestions.

@bradfitz bradfitz closed this as completed Nov 1, 2016
@golang golang locked and limited conversation to collaborators Nov 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants