golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.47k stars 17.59k forks source link

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

Open nkeonkeo opened 2 years ago

nkeonkeo commented 2 years ago

The accepted proposal is to add new methods to new.Dialer: https://github.com/golang/go/issues/49097#issuecomment-989057178

--

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 commented 2 years ago

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

nkeonkeo commented 2 years ago

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 commented 2 years ago

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.

rsc commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Any objections to https://github.com/golang/go/issues/49097#issuecomment-953157835 ?

rsc commented 2 years ago

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

rsc commented 2 years ago

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 commented 2 years ago

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

rsc commented 2 years ago

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

gopherbot commented 2 years ago

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

gopherbot commented 1 year ago

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

ShadowJonathan commented 5 months ago

Where is this proposal at? Its been accepted, but https://github.com/golang/go/pull/50534 has been closed.