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

proposal: net: add context-aware Dialer methods DialTCP, DialUDP, DialIP, DialUnix #49097

Open
nkeonkeo opened this issue Oct 21, 2021 · 8 comments
Open
Labels
Projects
Milestone

Comments

@nkeonkeo
Copy link

@nkeonkeo nkeonkeo commented Oct 21, 2021

Such like given below:

func DialTCP(network string, laddr, raddr *TCPAddr) (*TCPConn, error) {
	return DialTCPContext(context.Background(), network, laddr, raddr)
}
func DialTCPContext(ctx context.Context, network string, laddr, raddr *TCPAddr) (*TCPConn, error) {
	switch network {
	case "tcp", "tcp4", "tcp6":
	default:
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: raddr.opAddr(), Err: UnknownNetworkError(network)}
	}
	if raddr == nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: nil, Err: errMissingAddress}
	}
	sd := &sysDialer{network: network, address: raddr.String()}
	c, err := sd.dialTCP(ctx, laddr, raddr)
	if err != nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: raddr.opAddr(), Err: err}
	}
	return c, nil
}

func DialUDP(network string, laddr, raddr *UDPAddr) (*UDPConn, error) {
	return DialUDPContext(context.Background(), network, laddr, raddr)
}
func DialUDPContext(ctx context.Context, network string, laddr, raddr *UDPAddr) (*UDPConn, error) {
	switch network {
	case "udp", "udp4", "udp6":
	default:
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: raddr.opAddr(), Err: UnknownNetworkError(network)}
	}
	if raddr == nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: nil, Err: errMissingAddress}
	}
	sd := &sysDialer{network: network, address: raddr.String()}
	c, err := sd.dialUDP(ctx, laddr, raddr)
	if err != nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: raddr.opAddr(), Err: err}
	}
	return c, nil
}

func DialIP(network string, laddr, raddr *IPAddr) (*IPConn, error) {
	return DialIPContext(context.Background(), network, laddr, raddr)
}
func DialIPContext(ctx context.Context, network string, laddr, raddr *IPAddr) (*IPConn, error) {
	if raddr == nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: nil, Err: errMissingAddress}
	}
	sd := &sysDialer{network: network, address: raddr.String()}
	c, err := sd.dialIP(ctx, laddr, raddr)
	if err != nil {
		return nil, &OpError{Op: "dial", Net: network, Source: laddr.opAddr(), Addr: raddr.opAddr(), Err: err}
	}
	return c, nil
}
@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Oct 21, 2021

Could you give more information about the reason you want to add them and how they will be used? Thanks.

Loading

@nkeonkeo
Copy link
Author

@nkeonkeo nkeonkeo commented Oct 21, 2021

such as:

ctx, cancel := context.WithTimeout(context.Background(), 30000*time.Millisecond)
defer cancel()
taddr := &net.TCPAddr{}
c, err := net.DialTCPContext(ctx, "tcp", nil, taddr)
...

It can realize the timeout processing of DialTCP.

DialUDP and DialIP are similary like this.

Loading

@jimen0
Copy link

@jimen0 jimen0 commented Oct 22, 2021

Hi @nkeonkeo - Isn't that what https://pkg.go.dev/net#Dialer.DialContext does? What would your proposal allow that isn't possible as of today? Thanks!

Loading

@nkeonkeo
Copy link
Author

@nkeonkeo nkeonkeo commented Oct 22, 2021

Hi @nkeonkeo - Isn't that what https://pkg.go.dev/net#Dialer.DialContext does? What would your proposal allow that isn't possible as of today? Thanks!

Dialer.DialContext will resolve the address each time, it may cause errors like:

lookup xxxx.com on 223.x.x.x:53: read udp 180.x.x.x:7792->223.x.x.x:53: i/o timeout

Use net.DialTCP to dial net.TCPAddr can avoid such problems.

And I hope I can use net.DialTCP with context to deal with timeout problems.

Loading

@jimen0
Copy link

@jimen0 jimen0 commented Oct 22, 2021

Isn't that something you can accomplish by configuring your dialer's Resolver attribute to have a Dial logic of your choice? You could cache resolutions there. https://pkg.go.dev/net#Resolver

That Dial receives a context.

Dial func(ctx context.Context, network, address string) (Conn, error)

Let me know in case I missed something obvious. Regards! 😃

Loading

@ianlancetaylor ianlancetaylor changed the title net: add DialTCPContext, DialUDPContext, DialIPContext proposal: net: add DialTCPContext, DialUDPContext, DialIPContext Oct 22, 2021
@gopherbot gopherbot added this to the Proposal milestone Oct 22, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Oct 22, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Oct 27, 2021

We could add these to net.Dialer, which is what we are using instead of adding new context-aware versions of all the top-level functions. The specific methods we'd need to add would be:

DialTCP(context.Context, string, *TCPAddr, *TCPAddr) (*TCPConn, error)
DialUDP
DialIP
DialUnix

Note that all take contexts but don't say "Context" in the name.

Thoughts?

Loading

@rsc
Copy link
Contributor

@rsc rsc commented Oct 27, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

Loading

@rsc rsc moved this from Incoming to Active in Proposals Oct 27, 2021
@rsc rsc changed the title proposal: net: add DialTCPContext, DialUDPContext, DialIPContext proposal: net: add context-aware Dialer methods DialTCP, DialUDP, DialIP, DialUnix Nov 3, 2021
@rsc
Copy link
Contributor

@rsc rsc commented Nov 10, 2021

Any objections to #49097 (comment) ?

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants