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.

Sign up for GitHub

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

Jump to bottom

proposal: net: should provide an interface for dialing #9360

Open
tv42 opened this issue Dec 17, 2014 · 8 comments
Open

proposal: net: should provide an interface for dialing #9360

tv42 opened this issue Dec 17, 2014 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Milestone
Proposal

Comments

@tv42
Copy link

tv42 commented Dec 17, 2014

net.Dialer sounds like it's close to making transports pluggable the way e.g. http.Client / http.Transport behave, but reading further one realizes that net.Dialer is hardcoded to basically be net.Dial with timeouts. This makes e.g. crypto/tls DialWithDialer less useful; I can't just easily replace the whole transport layer with something that e.g. doesn't touch the host TCP stack (think unit tests, Tor, stream protocols constructed on top of UDP, and such).

Note that tls.DialWithDialer doesn't just dialer.Dial, it also accesses the Timeout field. By the time Go2 comes around, maybe /x/net/context will have grown into something that can unify this sort of behavior.

Of course, the workaround is to dial manually and use tls.Client, but then one is responsible for all the little details that are inside tls.DialWithDialer.

Naturally, I don't expect Go1 to change existing APIs, though something new might be workable. Sorry for the ramble, just felt like writing down my hopes of reusability being crushed.
@bradfitz bradfitz added the v2 A language change or incompatible library change label Dec 17, 2014
@bradfitz bradfitz changed the title Go2: net.Dialer seems like it should be an interface net: Dialer seems like it should be an interface Dec 17, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@egonelbre egonelbre mentioned this issue Sep 5, 2016
Closed
@sheerun
Copy link

sheerun commented May 26, 2017

It's the same issue as with Resolver field of default Dialer, it should be an interface

@rsc
Copy link
Contributor

rsc commented Jun 17, 2017

Dialer probably should continue to be a concrete type - the whole point is to expose control over the configuration settings in the struct. But any APIs that are written to take a Dialer only for the ability to Dial should probably take an interface that Dialer satisfies instead (or just a func).

@rsc rsc changed the title net: Dialer seems like it should be an interface proposal: net: Dialer seems like it should be an interface Jun 17, 2017
@ianlancetaylor ianlancetaylor changed the title proposal: net: Dialer seems like it should be an interface proposal: net: should provide an interface for dialing Jan 9, 2018
@ianlancetaylor
Copy link
Contributor

Agree with @rsc: we should have a concrete Dialer, but the net package could provide an interface (like io.Writer) that can be used to dial out.

@ianlancetaylor ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 9, 2018
@gm42
Copy link

gm42 commented Feb 16, 2018

This would allow "mocking" the TCP stack and reproducing low-level error conditions for other network packages (think about SSH control flow and other protocols), definitely a nice addition.

@navytux
Copy link
Contributor

navytux commented May 10, 2018

Data point: in the meantime some of us use interfaces like xnet.Networker to Dial/Listen with plain TCP or TLS/TCP or other networks in uniform way.

@ghost
Copy link

ghost commented Dec 24, 2018

I'd like to DialWithDialer using a dialer provided by ipsn/go-libtor and cretz/bine, or just a x/net/proxy.Dialer would already be a start.

Would a patch be welcome that defines a tls.Dialer interface and uses that for DialWithDialer, as @rsc suggests above? I guess the Timeout and Deadline usage could be guarded behind a type assertion?

It would be lovely to have tls/crypto decoupled from TCP! :)

@berkant
Copy link
Contributor

berkant commented Mar 11, 2019

Yeah, this thing would be good.

@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2019
@tzneal tzneal mentioned this issue Oct 13, 2022
Closed
@rg0now rg0now mentioned this issue Mar 25, 2023
Merged
@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed v2 A language change or incompatible library change NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 23, 2023
@ianlancetaylor ianlancetaylor modified the milestones: Unplanned, Proposal Aug 23, 2023
@ianlancetaylor
Copy link
Contributor

Looking at this issue again, it's rather vague. If someone can define a clear new API to implement in the net package or elsewhere, that would be helpful.

@brandond brandond mentioned this issue Dec 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Proposal
Projects
Proposals
Status: Incoming
Milestone
Proposal
Development

No branches or pull requests
9 participants
@bradfitz @rsc @sheerun @tv42 @navytux @ianlancetaylor @gopherbot @gm42 @berkant