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

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

Open
nkeonkeo opened this issue Oct 21, 2021 · 13 comments
Open

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

nkeonkeo opened this issue Oct 21, 2021 · 13 comments
Labels
Proposal Proposal-Accepted
Projects
Proposals (old)
Milestone
Backlog

Comments

@nkeonkeo
Copy link

nkeonkeo commented Oct 21, 2021 •
edited by ianlancetaylor

The accepted proposal is to add new methods to new.Dialer: #49097 (comment)

--

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
Member

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

@cherrymui cherrymui added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 21, 2021
@nkeonkeo
Copy link
Author

nkeonkeo commented Oct 21, 2021 •
edited

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.

@nkeonkeo
Copy link
Author

nkeonkeo commented Oct 22, 2021 •
edited

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.

@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 Proposal and removed Proposal WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Oct 22, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Oct 22, 2021
@rsc
Copy link
Contributor

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?

@rsc
Copy link
Contributor

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

@rsc rsc moved this from Incoming to Active in Proposals (old) 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
@ianlancetaylor ianlancetaylor mentioned this issue Nov 4, 2021
Closed
@rsc
Copy link
Contributor

rsc commented Nov 10, 2021

Any objections to #49097 (comment) ?

@bradfitz bradfitz mentioned this issue Nov 11, 2021
Closed
@rsc
Copy link
Contributor

rsc commented Dec 1, 2021

With the introduction of net/netip, we should probably make these new methods use those types (netip.AddrPort) instead of TCPAddr, UDPAddr.

@rsc rsc mentioned this issue Dec 1, 2021
Open
@rsc
Copy link
Contributor

rsc commented Dec 8, 2021

It sounds like we should be adding these methods on net.Dialer:

DialTCP(context.Context, string, netip.AddrPort, netip.AddrPort) (*TCPConn, error)
DialUDP(context.Context, string, netip.AddrPort, netip.AddrPort) (*TCPConn, error)
DialIP(context.Context, string, netip.Addr, netip.Addr) (*TCPConn, error)
DialUnix(context.Context, string, *UnixAddr, *UnixAddr) (*UnixConn, error)

Does anyone object to these?

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Dec 15, 2021
@rsc
Copy link
Contributor

rsc commented Dec 15, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Jan 5, 2022
@rsc
Copy link
Contributor

rsc commented Jan 5, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: net: add context-aware Dialer methods DialTCP, DialUDP, DialIP, DialUnix net: add context-aware Dialer methods DialTCP, DialUDP, DialIP, DialUnix Jan 5, 2022
@rsc rsc modified the milestones: Proposal, Backlog Jan 5, 2022
@moredure moredure mentioned this issue Jan 10, 2022
Closed
@gopherbot
Copy link

Change https://golang.org/cl/377155 mentions this issue: net: add DialUDPContext version of DialUDP

@ianlancetaylor ianlancetaylor mentioned this issue Apr 29, 2023
Open
@gopherbot
Copy link

Change https://go.dev/cl/490975 mentions this issue: net: context aware network Dialer.Dial functions

@ShadowJonathan
Copy link

ShadowJonathan commented Apr 26, 2024 •
edited

Where is this proposal at? Its been accepted, but #50534 has been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Accepted
Projects
Proposals
Status: Accepted
Proposals (old)
Accepted
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

net: add context-aware DialTCPContext, DialUDPContext, etc
6 participants
@rsc @ianlancetaylor @gopherbot @cherrymui @ShadowJonathan @nkeonkeo