Skip to content

refactor(netxlite): use *Netx for the system dialer #1249

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

Merged
merged 3 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions internal/netxlite/dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,17 @@ func NewDialerWithStdlibResolver(dl model.DebugLogger) model.Dialer {
return NewDialerWithResolver(dl, reso)
}

// NewDialerWithResolver is equivalent to calling WrapDialer with
// the dialer argument being equal to &DialerSystem{}.
// NewDialerWithResolver is equivalent to calling WrapDialer with a dialer using the
// the [*Netx] UnderlyingNetwork for dialing new connections.
func (netx *Netx) NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer {
return WrapDialer(dl, r, &dialerSystem{provider: netx.maybeCustomUnderlyingNetwork()}, w...)
}

// NewDialerWithResolver is equivalent to creating an empty [*Netx]
// and calling its NewDialerWithResolver method.
func NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer {
return WrapDialer(dl, r, &DialerSystem{}, w...)
netx := &Netx{Underlying: nil}
return netx.NewDialerWithResolver(dl, r, w...)
}

// WrapDialer wraps an existing Dialer to add extra functionality
Expand Down Expand Up @@ -142,24 +149,24 @@ func NewDialerWithoutResolver(dl model.DebugLogger, w ...model.DialerWrapper) mo
return NewDialerWithResolver(dl, &NullResolver{}, w...)
}

// DialerSystem is a model.Dialer that uses the stdlib's net.Dialer
// dialerSystem is a model.Dialer that uses the stdlib's net.Dialer
// to construct the new SimpleDialer used for dialing. This dialer has
// a fixed timeout for each connect operation equal to 15 seconds.
type DialerSystem struct {
type dialerSystem struct {
// provider is the OPTIONAL nil-safe [model.UnderlyingNetwork] provider.
provider *MaybeCustomUnderlyingNetwork
}

var _ model.Dialer = &DialerSystem{}
var _ model.Dialer = &dialerSystem{}

func (d *DialerSystem) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
func (d *dialerSystem) DialContext(ctx context.Context, network, address string) (net.Conn, error) {
p := d.provider.Get()
ctx, cancel := context.WithTimeout(ctx, p.DialTimeout())
defer cancel()
return p.DialContext(ctx, network, address)
}

func (d *DialerSystem) CloseIdleConnections() {
func (d *dialerSystem) CloseIdleConnections() {
// nothing to do here
}

Expand Down
18 changes: 9 additions & 9 deletions internal/netxlite/dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestNewDialerWithStdlibResolver(t *testing.T) {
t.Fatal("invalid logger")
}
errWrapper := logger.Dialer.(*dialerErrWrapper)
_ = errWrapper.Dialer.(*DialerSystem)
_ = errWrapper.Dialer.(*dialerSystem)
}

type extensionDialerFirst struct {
Expand Down Expand Up @@ -76,19 +76,19 @@ func TestNewDialer(t *testing.T) {
ext2 := logger.Dialer.(*extensionDialerSecond)
ext1 := ext2.Dialer.(*extensionDialerFirst)
errWrapper := ext1.Dialer.(*dialerErrWrapper)
_ = errWrapper.Dialer.(*DialerSystem)
_ = errWrapper.Dialer.(*dialerSystem)
})
}

func TestDialerSystem(t *testing.T) {
t.Run("CloseIdleConnections", func(t *testing.T) {
d := &DialerSystem{}
d := &dialerSystem{}
d.CloseIdleConnections() // to avoid missing coverage
})

t.Run("DialContext", func(t *testing.T) {
t.Run("with canceled context", func(t *testing.T) {
d := &DialerSystem{}
d := &dialerSystem{}
ctx, cancel := context.WithCancel(context.Background())
cancel() // immediately!
conn, err := d.DialContext(ctx, "tcp", "8.8.8.8:443")
Expand All @@ -108,7 +108,7 @@ func TestDialerSystem(t *testing.T) {
},
MockDialContext: defaultTp.DialContext,
}
d := &DialerSystem{provider: &MaybeCustomUnderlyingNetwork{tp}}
d := &dialerSystem{provider: &MaybeCustomUnderlyingNetwork{tp}}
ctx := context.Background()
start := time.Now()
conn, err := d.DialContext(ctx, "tcp", "dns.google:443")
Expand All @@ -134,7 +134,7 @@ func TestDialerSystem(t *testing.T) {
return nil, expected
},
}
d := &DialerSystem{provider: &MaybeCustomUnderlyingNetwork{proxy}}
d := &dialerSystem{provider: &MaybeCustomUnderlyingNetwork{proxy}}
conn, err := d.DialContext(context.Background(), "tcp", "dns.google:443")
if conn != nil {
t.Fatal("unexpected conn")
Expand All @@ -150,7 +150,7 @@ func TestDialerResolverWithTracing(t *testing.T) {
t.Run("DialContext", func(t *testing.T) {
t.Run("fails without a port", func(t *testing.T) {
d := &dialerResolverWithTracing{
Dialer: &DialerSystem{},
Dialer: &dialerSystem{},
Resolver: NewUnwrappedStdlibResolver(),
}
const missingPort = "ooni.nu"
Expand Down Expand Up @@ -497,7 +497,7 @@ func TestDialerResolverWithTracing(t *testing.T) {
t.Run("lookupHost", func(t *testing.T) {
t.Run("handles addresses correctly", func(t *testing.T) {
dialer := &dialerResolverWithTracing{
Dialer: &DialerSystem{},
Dialer: &dialerSystem{},
Resolver: &NullResolver{},
}
addrs, err := dialer.lookupHost(context.Background(), "1.1.1.1")
Expand All @@ -511,7 +511,7 @@ func TestDialerResolverWithTracing(t *testing.T) {

t.Run("fails correctly on lookup error", func(t *testing.T) {
dialer := &dialerResolverWithTracing{
Dialer: &DialerSystem{},
Dialer: &dialerSystem{},
Resolver: &NullResolver{},
}
ctx := context.Background()
Expand Down
6 changes: 0 additions & 6 deletions internal/netxlite/netx.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ func (netx *Netx) maybeCustomUnderlyingNetwork() *MaybeCustomUnderlyingNetwork {
return &MaybeCustomUnderlyingNetwork{netx.Underlying}
}

// NewDialerWithResolver is like [netxlite.NewDialerWithResolver] but the constructed [model.Dialer]
// uses the [model.UnderlyingNetwork] configured inside the [Netx] structure.
func (n *Netx) NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer {
return WrapDialer(dl, r, &DialerSystem{provider: n.maybeCustomUnderlyingNetwork()}, w...)
}

// NewUDPListener is like [netxlite.NewUDPListener] but the constructed [model.UDPListener]
// uses the [model.UnderlyingNetwork] configured inside the [Netx] structure.
func (n *Netx) NewUDPListener() model.UDPListener {
Expand Down
2 changes: 1 addition & 1 deletion internal/netxlite/resolvercore.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (netx *Netx) NewStdlibResolver(logger model.DebugLogger) model.Resolver {
}

// NewStdlibResolver is equivalent to creating an empty [*Netx]
// and callings its NewStdlibResolver method.
// and calling its NewStdlibResolver method.
func NewStdlibResolver(logger model.DebugLogger) model.Resolver {
netx := &Netx{Underlying: nil}
return netx.NewStdlibResolver(logger)
Expand Down
2 changes: 1 addition & 1 deletion internal/netxlite/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ func TestTLSDialer(t *testing.T) {
t.Run("failure dialing", func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel() // immediately fail
dialer := tlsDialer{Dialer: &DialerSystem{}}
dialer := tlsDialer{Dialer: &dialerSystem{}}
conn, err := dialer.DialTLSContext(ctx, "tcp", "www.google.com:443")
if err == nil || !strings.HasSuffix(err.Error(), "operation was canceled") {
t.Fatal("not the error we expected", err)
Expand Down