diff --git a/internal/engine/legacy/netx/resolver.go b/internal/engine/legacy/netx/resolver.go index cb02a5b..a522f7d 100644 --- a/internal/engine/legacy/netx/resolver.go +++ b/internal/engine/legacy/netx/resolver.go @@ -12,6 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -150,7 +151,7 @@ func ChainResolvers(primary, secondary modelx.DNSResolver) modelx.DNSResolver { } func resolverWrapResolver(r resolver.Resolver) resolver.EmitterResolver { - return resolver.EmitterResolver{Resolver: resolver.ErrorWrapperResolver{Resolver: r}} + return resolver.EmitterResolver{Resolver: &errorsx.ErrorWrapperResolver{Resolver: r}} } func resolverWrapTransport(txp resolver.RoundTripper) resolver.EmitterResolver { diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index cc66532..a2cccd4 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -132,7 +132,7 @@ func NewResolver(config Config) Resolver { if config.BogonIsError { r = resolver.BogonResolver{Resolver: r} } - r = resolver.ErrorWrapperResolver{Resolver: r} + r = &errorsx.ErrorWrapperResolver{Resolver: r} if config.Logger != nil { r = &netxlite.ResolverLogger{Logger: config.Logger, Resolver: r} } diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index a15ed8e..21506e1 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -24,7 +24,7 @@ func TestNewResolverVanilla(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -48,7 +48,7 @@ func TestNewResolverSpecificResolver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -70,7 +70,7 @@ func TestNewResolverWithBogonFilter(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -103,7 +103,7 @@ func TestNewResolverWithLogging(t *testing.T) { if lr.Logger != log.Log { t.Fatal("not the logger we expected") } - ewr, ok := lr.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := lr.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -133,7 +133,7 @@ func TestNewResolverWithSaver(t *testing.T) { if sr.Saver != saver { t.Fatal("not the saver we expected") } - ewr, ok := sr.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := sr.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -155,7 +155,7 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -186,7 +186,7 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := ir.Resolver.(resolver.ErrorWrapperResolver) + ewr, ok := ir.Resolver.(*errorsx.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } diff --git a/internal/engine/netx/resolver/errorwrapper.go b/internal/engine/netx/resolver/errorwrapper.go deleted file mode 100644 index 9a34cc5..0000000 --- a/internal/engine/netx/resolver/errorwrapper.go +++ /dev/null @@ -1,25 +0,0 @@ -package resolver - -import ( - "context" - - "github.com/ooni/probe-cli/v3/internal/errorsx" -) - -// ErrorWrapperResolver is a Resolver that knows about wrapping errors. -type ErrorWrapperResolver struct { - Resolver -} - -// LookupHost implements Resolver.LookupHost -func (r ErrorWrapperResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { - addrs, err := r.Resolver.LookupHost(ctx, hostname) - err = errorsx.SafeErrWrapperBuilder{ - Classifier: errorsx.ClassifyResolveFailure, - Error: err, - Operation: errorsx.ResolveOperation, - }.MaybeBuild() - return addrs, err -} - -var _ Resolver = ErrorWrapperResolver{} diff --git a/internal/engine/netx/resolver/errorwrapper_test.go b/internal/engine/netx/resolver/errorwrapper_test.go deleted file mode 100644 index 7b900d3..0000000 --- a/internal/engine/netx/resolver/errorwrapper_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package resolver_test - -import ( - "context" - "errors" - "testing" - - "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" - "github.com/ooni/probe-cli/v3/internal/errorsx" -) - -func TestErrorWrapperSuccess(t *testing.T) { - orig := []string{"8.8.8.8"} - r := resolver.ErrorWrapperResolver{ - Resolver: resolver.NewFakeResolverWithResult(orig), - } - addrs, err := r.LookupHost(context.Background(), "dns.google.com") - if err != nil { - t.Fatal(err) - } - if len(addrs) != len(orig) || addrs[0] != orig[0] { - t.Fatal("not the result we expected") - } -} - -func TestErrorWrapperFailure(t *testing.T) { - r := resolver.ErrorWrapperResolver{ - Resolver: resolver.NewFakeResolverThatFails(), - } - ctx := context.Background() - addrs, err := r.LookupHost(ctx, "dns.google.com") - if addrs != nil { - t.Fatal("expected nil addr here") - } - var errWrapper *errorsx.ErrWrapper - if !errors.As(err, &errWrapper) { - t.Fatal("cannot properly cast the returned error") - } - if errWrapper.Failure != errorsx.FailureDNSNXDOMAINError { - t.Fatal("unexpected failure") - } - if errWrapper.Operation != errorsx.ResolveOperation { - t.Fatal("unexpected Operation") - } -} diff --git a/internal/errorsx/dialer_test.go b/internal/errorsx/dialer_test.go index bbd2920..a870c47 100644 --- a/internal/errorsx/dialer_test.go +++ b/internal/errorsx/dialer_test.go @@ -10,7 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxmocks" ) -func TestErrorWrapperFailure(t *testing.T) { +func TestErrorWrapperDialerFailure(t *testing.T) { ctx := context.Background() d := &ErrorWrapperDialer{Dialer: &netxmocks.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { @@ -40,7 +40,7 @@ func errorWrapperCheckErr(t *testing.T, err error, op string) { } } -func TestErrorWrapperSuccess(t *testing.T) { +func TestErrorWrapperDialerSuccess(t *testing.T) { ctx := context.Background() d := &ErrorWrapperDialer{Dialer: &netxmocks.Dialer{ MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { diff --git a/internal/errorsx/resolver.go b/internal/errorsx/resolver.go new file mode 100644 index 0000000..459ce83 --- /dev/null +++ b/internal/errorsx/resolver.go @@ -0,0 +1,52 @@ +package errorsx + +import "context" + +// Resolver is a DNS resolver. The *net.Resolver used by Go implements +// this interface, but other implementations are possible. +type Resolver interface { + // LookupHost resolves a hostname to a list of IP addresses. + LookupHost(ctx context.Context, hostname string) (addrs []string, err error) +} + +// ErrorWrapperResolver is a Resolver that knows about wrapping errors. +type ErrorWrapperResolver struct { + Resolver +} + +var _ Resolver = &ErrorWrapperResolver{} + +// LookupHost implements Resolver.LookupHost +func (r *ErrorWrapperResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { + addrs, err := r.Resolver.LookupHost(ctx, hostname) + err = SafeErrWrapperBuilder{ + Classifier: ClassifyResolveFailure, + Error: err, + Operation: ResolveOperation, + }.MaybeBuild() + return addrs, err +} + +type resolverNetworker interface { + Network() string +} + +// Network implements Resolver.Network. +func (r *ErrorWrapperResolver) Network() string { + if rn, ok := r.Resolver.(resolverNetworker); ok { + return rn.Network() + } + return "errorWrapper" +} + +type resolverAddresser interface { + Address() string +} + +// Address implements Resolver.Address. +func (r *ErrorWrapperResolver) Address() string { + if ra, ok := r.Resolver.(resolverAddresser); ok { + return ra.Address() + } + return "" +} diff --git a/internal/errorsx/resolver_test.go b/internal/errorsx/resolver_test.go new file mode 100644 index 0000000..b65caad --- /dev/null +++ b/internal/errorsx/resolver_test.go @@ -0,0 +1,80 @@ +package errorsx + +import ( + "context" + "errors" + "net" + "testing" + + "github.com/ooni/probe-cli/v3/internal/netxmocks" +) + +func TestErrorWrapperResolverSuccess(t *testing.T) { + orig := []string{"8.8.8.8"} + r := &ErrorWrapperResolver{ + Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return orig, nil + }, + }, + } + addrs, err := r.LookupHost(context.Background(), "dns.google.com") + if err != nil { + t.Fatal(err) + } + if len(addrs) != len(orig) || addrs[0] != orig[0] { + t.Fatal("not the result we expected") + } +} + +func TestErrorWrapperResolverFailure(t *testing.T) { + r := &ErrorWrapperResolver{ + Resolver: &netxmocks.Resolver{ + MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { + return nil, errors.New("no such host") + }, + }, + } + ctx := context.Background() + addrs, err := r.LookupHost(ctx, "dns.google.com") + if addrs != nil { + t.Fatal("expected nil addr here") + } + var errWrapper *ErrWrapper + if !errors.As(err, &errWrapper) { + t.Fatal("cannot properly cast the returned error") + } + if errWrapper.Failure != FailureDNSNXDOMAINError { + t.Fatal("unexpected failure") + } + if errWrapper.Operation != ResolveOperation { + t.Fatal("unexpected Operation") + } +} + +func TestErrorWrapperResolverChildNetworkAddress(t *testing.T) { + r := &ErrorWrapperResolver{Resolver: &netxmocks.Resolver{ + MockNetwork: func() string { + return "udp" + }, + MockAddress: func() string { + return "8.8.8.8:53" + }, + }} + if r.Network() != "udp" { + t.Fatal("invalid Network") + } + if r.Address() != "8.8.8.8:53" { + t.Fatal("invalid Address") + } +} + +func TestErrorWrapperResolverNoChildNetworkAddress(t *testing.T) { + r := &ErrorWrapperResolver{Resolver: &net.Resolver{}} + if r.Network() != "errorWrapper" { + t.Fatal("invalid Network") + } + if r.Address() != "" { + t.Fatal("invalid Address") + } +}