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

Comments

Projects
None yet
5 participants
@rogpeppe
Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2016

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 label Oct 7, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

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 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.
You can’t perform that action at this time.