diff --git a/internal/engine/experiment/hhfm/hhfm.go b/internal/engine/experiment/hhfm/hhfm.go index 7d916eb..59fddf4 100644 --- a/internal/engine/experiment/hhfm/hhfm.go +++ b/internal/engine/experiment/hhfm/hhfm.go @@ -16,7 +16,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/httpheader" - errorsxlegacy "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/model" @@ -180,8 +179,9 @@ func Transact(txp Transport, req *http.Request, callbacks model.ExperimentCallbacks) (*http.Response, []byte, error) { // make sure that we return a wrapped error here resp, data, err := transact(txp, req, callbacks) - err = errorsxlegacy.SafeErrWrapperBuilder{ - Error: err, Operation: netxlite.TopLevelOperation}.MaybeBuild() + if err != nil { + err = netxlite.NewTopLevelGenericErrWrapper(err) + } return resp, data, err } diff --git a/internal/engine/experiment/stunreachability/stunreachability.go b/internal/engine/experiment/stunreachability/stunreachability.go index a19b263..8caf333 100644 --- a/internal/engine/experiment/stunreachability/stunreachability.go +++ b/internal/engine/experiment/stunreachability/stunreachability.go @@ -11,12 +11,12 @@ import ( "net/url" "time" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/pion/stun" ) @@ -60,10 +60,10 @@ func (m *Measurer) ExperimentVersion() string { } func wrap(err error) error { - return errorsx.SafeErrWrapperBuilder{ - Error: err, - Operation: "stun", - }.MaybeBuild() + if err != nil { + return netxlite.NewTopLevelGenericErrWrapper(err) + } + return nil } // errStunMissingInput means that the user did not provide any input diff --git a/internal/engine/experiment/urlgetter/getter.go b/internal/engine/experiment/urlgetter/getter.go index 2292a71..eab383a 100644 --- a/internal/engine/experiment/urlgetter/getter.go +++ b/internal/engine/experiment/urlgetter/getter.go @@ -6,7 +6,6 @@ import ( "net/url" "time" - legacyerrorsx "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/model" @@ -55,10 +54,9 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) { tk, err := g.get(ctx, saver) // Make sure we have an operation in cases where we fail before // hitting our httptransport that does error wrapping. - err = legacyerrorsx.SafeErrWrapperBuilder{ - Error: err, - Operation: netxlite.TopLevelOperation, - }.MaybeBuild() + if err != nil { + err = netxlite.NewTopLevelGenericErrWrapper(err) + } tk.FailedOperation = archival.NewFailedOperation(err) tk.Failure = archival.NewFailure(err) events := saver.Read() diff --git a/internal/engine/experiment/webconnectivity/control.go b/internal/engine/experiment/webconnectivity/control.go index 0ff4a7c..f75a912 100644 --- a/internal/engine/experiment/webconnectivity/control.go +++ b/internal/engine/experiment/webconnectivity/control.go @@ -4,7 +4,6 @@ import ( "context" "github.com/ooni/probe-cli/v3/internal/engine/geolocate" - legacyerrorsx "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/httpx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" @@ -61,10 +60,10 @@ func Control( } sess.Logger().Infof("control for %s...", creq.HTTPRequest) // make sure error is wrapped - err = legacyerrorsx.SafeErrWrapperBuilder{ - Error: clnt.WithBodyLogging().Build().PostJSON(ctx, "/", creq, &out), - Operation: netxlite.TopLevelOperation, - }.MaybeBuild() + err = clnt.WithBodyLogging().Build().PostJSON(ctx, "/", creq, &out) + if err != nil { + err = netxlite.NewTopLevelGenericErrWrapper(err) + } sess.Logger().Infof("control for %s... %+v", creq.HTTPRequest, err) (&out.DNS).FillASNs(sess) return diff --git a/internal/engine/experiment/websteps/control.go b/internal/engine/experiment/websteps/control.go index 8e9ebf1..c1c3f0f 100644 --- a/internal/engine/experiment/websteps/control.go +++ b/internal/engine/experiment/websteps/control.go @@ -3,7 +3,6 @@ package websteps import ( "context" - errorsxlegacy "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/httpx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" @@ -19,9 +18,9 @@ func Control( Logger: sess.Logger(), } // make sure error is wrapped - err = errorsxlegacy.SafeErrWrapperBuilder{ - Error: clnt.WithBodyLogging().Build().PostJSON(ctx, resourcePath, creq, &out), - Operation: netxlite.TopLevelOperation, - }.MaybeBuild() + err = clnt.WithBodyLogging().Build().PostJSON(ctx, resourcePath, creq, &out) + if err != nil { + err = netxlite.NewTopLevelGenericErrWrapper(err) + } return } diff --git a/internal/engine/experiment/websteps/factory.go b/internal/engine/experiment/websteps/factory.go index 44cc5c3..d75fc52 100644 --- a/internal/engine/experiment/websteps/factory.go +++ b/internal/engine/experiment/websteps/factory.go @@ -12,8 +12,6 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go/http3" oohttp "github.com/ooni/oohttp" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" - "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" @@ -34,28 +32,28 @@ func NewRequest(ctx context.Context, URL *url.URL, headers http.Header) *http.Re // NewDialerResolver contructs a new dialer for TCP connections, // with default, errorwrapping and resolve functionalities -func NewDialerResolver(resolver netxlite.ResolverLegacy) netxlite.DialerLegacy { - var d netxlite.DialerLegacy = netxlite.DefaultDialer - d = &errorsx.ErrorWrapperDialer{Dialer: d} +func NewDialerResolver(resolver netxlite.ResolverLegacy) model.Dialer { + var d model.Dialer = netxlite.DefaultDialer + d = &netxlite.ErrorWrapperDialer{Dialer: d} d = &netxlite.DialerResolver{ Resolver: netxlite.NewResolverLegacyAdapter(resolver), - Dialer: netxlite.NewDialerLegacyAdapter(d), + Dialer: d, } return d } // NewQUICDialerResolver creates a new QUICDialerResolver // with default, errorwrapping and resolve functionalities -func NewQUICDialerResolver(resolver netxlite.ResolverLegacy) netxlite.QUICContextDialer { - var ql quicdialer.QUICListener = &netxlite.QUICListenerStdlib{} - ql = &errorsx.ErrorWrapperQUICListener{QUICListener: ql} - var dialer netxlite.QUICContextDialer = &netxlite.QUICDialerQUICGo{ +func NewQUICDialerResolver(resolver netxlite.ResolverLegacy) model.QUICDialer { + var ql model.QUICListener = &netxlite.QUICListenerStdlib{} + ql = &netxlite.ErrorWrapperQUICListener{QUICListener: ql} + var dialer model.QUICDialer = &netxlite.QUICDialerQUICGo{ QUICListener: ql, } - dialer = &errorsx.ErrorWrapperQUICDialer{Dialer: dialer} + dialer = &netxlite.ErrorWrapperQUICDialer{QUICDialer: dialer} dialer = &netxlite.QUICDialerResolver{ Resolver: netxlite.NewResolverLegacyAdapter(resolver), - Dialer: netxlite.NewQUICDialerFromContextDialerAdapter(dialer), + Dialer: dialer, } return dialer } diff --git a/internal/engine/legacy/errorsx/dialer.go b/internal/engine/legacy/errorsx/dialer.go deleted file mode 100644 index 9de2ac5..0000000 --- a/internal/engine/legacy/errorsx/dialer.go +++ /dev/null @@ -1,79 +0,0 @@ -package errorsx - -import ( - "context" - "net" - - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// Dialer establishes network connections. -type Dialer interface { - // DialContext behaves like net.Dialer.DialContext. - DialContext(ctx context.Context, network, address string) (net.Conn, error) -} - -// ErrorWrapperDialer is a dialer that performs error wrapping. The connection -// returned by the DialContext function will also perform error wrapping. -type ErrorWrapperDialer struct { - // Dialer is the underlying dialer. - Dialer -} - -// DialContext implements Dialer.DialContext. -func (d *ErrorWrapperDialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { - conn, err := d.Dialer.DialContext(ctx, network, address) - if err != nil { - return nil, SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyGenericError, - Operation: netxlite.ConnectOperation, - Error: err, - }.MaybeBuild() - } - return &errorWrapperConn{Conn: conn}, nil -} - -// errorWrapperConn is a net.Conn that performs error wrapping. -type errorWrapperConn struct { - // Conn is the underlying connection. - net.Conn -} - -// Read implements net.Conn.Read. -func (c *errorWrapperConn) Read(b []byte) (int, error) { - count, err := c.Conn.Read(b) - if err != nil { - return 0, SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyGenericError, - Operation: netxlite.ReadOperation, - Error: err, - }.MaybeBuild() - } - return count, nil -} - -// Write implements net.Conn.Write. -func (c *errorWrapperConn) Write(b []byte) (int, error) { - count, err := c.Conn.Write(b) - if err != nil { - return 0, SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyGenericError, - Operation: netxlite.WriteOperation, - Error: err, - }.MaybeBuild() - } - return count, nil -} - -// Close implements net.Conn.Close. -func (c *errorWrapperConn) Close() error { - err := c.Conn.Close() - if err != nil { - return SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyGenericError, - Operation: netxlite.CloseOperation, - Error: err, - }.MaybeBuild() - } - return nil -} diff --git a/internal/engine/legacy/errorsx/dialer_test.go b/internal/engine/legacy/errorsx/dialer_test.go deleted file mode 100644 index 48f1a21..0000000 --- a/internal/engine/legacy/errorsx/dialer_test.go +++ /dev/null @@ -1,189 +0,0 @@ -package errorsx - -import ( - "context" - "errors" - "io" - "net" - "testing" - - "github.com/ooni/probe-cli/v3/internal/model/mocks" - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -func TestErrorWrapperDialerFailure(t *testing.T) { - ctx := context.Background() - d := &ErrorWrapperDialer{Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return nil, io.EOF - }, - }} - conn, err := d.DialContext(ctx, "tcp", "www.google.com:443") - var ew *netxlite.ErrWrapper - if !errors.As(err, &ew) { - t.Fatal("cannot convert to ErrWrapper") - } - if ew.Operation != netxlite.ConnectOperation { - t.Fatal("unexpected operation", ew.Operation) - } - if ew.Failure != netxlite.FailureEOFError { - t.Fatal("unexpected failure", ew.Failure) - } - if !errors.Is(ew.WrappedErr, io.EOF) { - t.Fatal("unexpected underlying error", ew.WrappedErr) - } - if conn != nil { - t.Fatal("expected a nil conn here") - } -} - -func TestErrorWrapperDialerSuccess(t *testing.T) { - origConn := &net.TCPConn{} - ctx := context.Background() - d := &ErrorWrapperDialer{Dialer: &mocks.Dialer{ - MockDialContext: func(ctx context.Context, network string, address string) (net.Conn, error) { - return origConn, nil - }, - }} - conn, err := d.DialContext(ctx, "tcp", "www.google.com:443") - if err != nil { - t.Fatal(err) - } - ewConn, ok := conn.(*errorWrapperConn) - if !ok { - t.Fatal("cannot cast to errorWrapperConn") - } - if ewConn.Conn != origConn { - t.Fatal("not the connection we expected") - } -} - -func TestErrorWrapperConnReadFailure(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockRead: func(b []byte) (int, error) { - return 0, io.EOF - }, - }, - } - buf := make([]byte, 1024) - cnt, err := c.Read(buf) - var ew *netxlite.ErrWrapper - if !errors.As(err, &ew) { - t.Fatal("cannot cast error to ErrWrapper") - } - if ew.Operation != netxlite.ReadOperation { - t.Fatal("invalid operation", ew.Operation) - } - if ew.Failure != netxlite.FailureEOFError { - t.Fatal("invalid failure", ew.Failure) - } - if !errors.Is(ew.WrappedErr, io.EOF) { - t.Fatal("invalid wrapped error", ew.WrappedErr) - } - if cnt != 0 { - t.Fatal("expected zero here", cnt) - } -} - -func TestErrorWrapperConnReadSuccess(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockRead: func(b []byte) (int, error) { - return len(b), nil - }, - }, - } - buf := make([]byte, 1024) - cnt, err := c.Read(buf) - if err != nil { - t.Fatal(err) - } - if cnt != len(buf) { - t.Fatal("expected len(buf) here", cnt) - } -} - -func TestErrorWrapperConnWriteFailure(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockWrite: func(b []byte) (int, error) { - return 0, io.EOF - }, - }, - } - buf := make([]byte, 1024) - cnt, err := c.Write(buf) - var ew *netxlite.ErrWrapper - if !errors.As(err, &ew) { - t.Fatal("cannot cast error to ErrWrapper") - } - if ew.Operation != netxlite.WriteOperation { - t.Fatal("invalid operation", ew.Operation) - } - if ew.Failure != netxlite.FailureEOFError { - t.Fatal("invalid failure", ew.Failure) - } - if !errors.Is(ew.WrappedErr, io.EOF) { - t.Fatal("invalid wrapped error", ew.WrappedErr) - } - if cnt != 0 { - t.Fatal("expected zero here", cnt) - } -} - -func TestErrorWrapperConnWriteSuccess(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockWrite: func(b []byte) (int, error) { - return len(b), nil - }, - }, - } - buf := make([]byte, 1024) - cnt, err := c.Write(buf) - if err != nil { - t.Fatal(err) - } - if cnt != len(buf) { - t.Fatal("expected len(buf) here", cnt) - } -} - -func TestErrorWrapperConnCloseFailure(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockClose: func() error { - return io.EOF - }, - }, - } - err := c.Close() - var ew *netxlite.ErrWrapper - if !errors.As(err, &ew) { - t.Fatal("cannot cast error to ErrWrapper") - } - if ew.Operation != netxlite.CloseOperation { - t.Fatal("invalid operation", ew.Operation) - } - if ew.Failure != netxlite.FailureEOFError { - t.Fatal("invalid failure", ew.Failure) - } - if !errors.Is(ew.WrappedErr, io.EOF) { - t.Fatal("invalid wrapped error", ew.WrappedErr) - } -} - -func TestErrorWrapperConnCloseSuccess(t *testing.T) { - c := &errorWrapperConn{ - Conn: &mocks.Conn{ - MockClose: func() error { - return nil - }, - }, - } - err := c.Close() - if err != nil { - t.Fatal(err) - } -} diff --git a/internal/engine/legacy/errorsx/errorsx.go b/internal/engine/legacy/errorsx/errorsx.go deleted file mode 100644 index 45d5b28..0000000 --- a/internal/engine/legacy/errorsx/errorsx.go +++ /dev/null @@ -1,70 +0,0 @@ -// Package errorsx contains error extensions. -package errorsx - -import ( - "errors" - - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// SafeErrWrapperBuilder contains a builder for ErrWrapper that -// is safe, i.e., behaves correctly when the error is nil. -type SafeErrWrapperBuilder struct { - // Error is the error, if any - Error error - - // Classifier is the local error to string classifier. When there is no - // configured classifier we will use the generic classifier. - Classifier func(err error) string - - // Operation is the operation that failed - Operation string -} - -// MaybeBuild builds a new ErrWrapper, if b.Error is not nil, and returns -// a nil error value, instead, if b.Error is nil. -func (b SafeErrWrapperBuilder) MaybeBuild() (err error) { - if b.Error != nil { - classifier := b.Classifier - if classifier == nil { - classifier = netxlite.ClassifyGenericError - } - err = &netxlite.ErrWrapper{ - Failure: classifier(b.Error), - Operation: toOperationString(b.Error, b.Operation), - WrappedErr: b.Error, - } - } - return -} - -func toOperationString(err error, operation string) string { - var errwrapper *netxlite.ErrWrapper - if errors.As(err, &errwrapper) { - // Basically, as explained in ErrWrapper docs, let's - // keep the child major operation, if any. - if errwrapper.Operation == netxlite.ConnectOperation { - return errwrapper.Operation - } - if errwrapper.Operation == netxlite.HTTPRoundTripOperation { - return errwrapper.Operation - } - if errwrapper.Operation == netxlite.ResolveOperation { - return errwrapper.Operation - } - if errwrapper.Operation == netxlite.TLSHandshakeOperation { - return errwrapper.Operation - } - if errwrapper.Operation == netxlite.QUICHandshakeOperation { - return errwrapper.Operation - } - if errwrapper.Operation == "quic_handshake_start" { - return netxlite.QUICHandshakeOperation - } - if errwrapper.Operation == "quic_handshake_done" { - return netxlite.QUICHandshakeOperation - } - // FALLTHROUGH - } - return operation -} diff --git a/internal/engine/legacy/errorsx/errorsx_test.go b/internal/engine/legacy/errorsx/errorsx_test.go deleted file mode 100644 index 7311b03..0000000 --- a/internal/engine/legacy/errorsx/errorsx_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package errorsx - -import ( - "errors" - "testing" - - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -func TestMaybeBuildFactory(t *testing.T) { - err := SafeErrWrapperBuilder{ - Error: errors.New("mocked error"), - }.MaybeBuild() - var target *netxlite.ErrWrapper - if errors.As(err, &target) == false { - t.Fatal("not the expected error type") - } - if target.Failure != "unknown_failure: mocked error" { - t.Fatal("the failure string is wrong") - } - if target.WrappedErr.Error() != "mocked error" { - t.Fatal("the wrapped error is wrong") - } -} - -func TestToOperationString(t *testing.T) { - t.Run("for connect", func(t *testing.T) { - // You're doing HTTP and connect fails. You want to know - // that connect failed not that HTTP failed. - err := &netxlite.ErrWrapper{Operation: netxlite.ConnectOperation} - if toOperationString(err, netxlite.HTTPRoundTripOperation) != netxlite.ConnectOperation { - t.Fatal("unexpected result") - } - }) - t.Run("for http_round_trip", func(t *testing.T) { - // You're doing DoH and something fails inside HTTP. You want - // to know about the internal HTTP error, not resolve. - err := &netxlite.ErrWrapper{Operation: netxlite.HTTPRoundTripOperation} - if toOperationString(err, netxlite.ResolveOperation) != netxlite.HTTPRoundTripOperation { - t.Fatal("unexpected result") - } - }) - t.Run("for resolve", func(t *testing.T) { - // You're doing HTTP and the DNS fails. You want to - // know that resolve failed. - err := &netxlite.ErrWrapper{Operation: netxlite.ResolveOperation} - if toOperationString(err, netxlite.HTTPRoundTripOperation) != netxlite.ResolveOperation { - t.Fatal("unexpected result") - } - }) - t.Run("for tls_handshake", func(t *testing.T) { - // You're doing HTTP and the TLS handshake fails. You want - // to know about a TLS handshake error. - err := &netxlite.ErrWrapper{Operation: netxlite.TLSHandshakeOperation} - if toOperationString(err, netxlite.HTTPRoundTripOperation) != netxlite.TLSHandshakeOperation { - t.Fatal("unexpected result") - } - }) - t.Run("for minor operation", func(t *testing.T) { - // You just noticed that TLS handshake failed and you - // have a child error telling you that read failed. Here - // you want to know about a TLS handshake error. - err := &netxlite.ErrWrapper{Operation: netxlite.ReadOperation} - if toOperationString(err, netxlite.TLSHandshakeOperation) != netxlite.TLSHandshakeOperation { - t.Fatal("unexpected result") - } - }) - t.Run("for quic_handshake", func(t *testing.T) { - // You're doing HTTP and the TLS handshake fails. You want - // to know about a TLS handshake error. - err := &netxlite.ErrWrapper{Operation: netxlite.QUICHandshakeOperation} - if toOperationString(err, netxlite.HTTPRoundTripOperation) != netxlite.QUICHandshakeOperation { - t.Fatal("unexpected result") - } - }) -} diff --git a/internal/engine/legacy/errorsx/integration_test.go b/internal/engine/legacy/errorsx/integration_test.go deleted file mode 100644 index 3c41266..0000000 --- a/internal/engine/legacy/errorsx/integration_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package errorsx_test - -import ( - "context" - "crypto/tls" - "testing" - - "github.com/lucas-clemente/quic-go" - errorsxlegacy "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" - "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/netxlite/quictesting" -) - -func TestErrorWrapperQUICDialerFailure(t *testing.T) { - nextprotos := []string{"h3"} - servername := "example.com" - tlsConf := &tls.Config{ - NextProtos: nextprotos, - ServerName: servername, - } - - dlr := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ - QUICListener: &netxlite.QUICListenerStdlib{}, - }} - sess, err := dlr.DialContext(context.Background(), "udp", - quictesting.Endpoint("443"), tlsConf, &quic.Config{}) - if err == nil { - t.Fatal("expected an error here") - } - if sess != nil { - t.Fatal("expected nil sess here") - } - if err.Error() != netxlite.FailureSSLFailedHandshake { - t.Fatal("unexpected failure", err.Error()) - } -} - -func TestErrorWrapperQUICDialerSuccess(t *testing.T) { - ctx := context.Background() - tlsConf := &tls.Config{ - NextProtos: []string{"h3"}, - ServerName: quictesting.Domain, - } - d := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ - QUICListener: &netxlite.QUICListenerStdlib{}, - }} - sess, err := d.DialContext(ctx, "udp", quictesting.Endpoint("443"), tlsConf, &quic.Config{}) - if err != nil { - t.Fatal(err) - } - if sess == nil { - t.Fatal("expected non-nil sess here") - } -} diff --git a/internal/engine/legacy/errorsx/quic.go b/internal/engine/legacy/errorsx/quic.go deleted file mode 100644 index b1e5eb7..0000000 --- a/internal/engine/legacy/errorsx/quic.go +++ /dev/null @@ -1,98 +0,0 @@ -package errorsx - -import ( - "context" - "crypto/tls" - "net" - - "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// QUICContextDialer is a dialer for QUIC using Context. -type QUICContextDialer interface { - // DialContext establishes a new QUIC session using the given - // network and address. The tlsConfig and the quicConfig arguments - // MUST NOT be nil. Returns either the session or an error. - DialContext(ctx context.Context, network, address string, - tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) -} - -// QUICListener listens for QUIC connections. -type QUICListener interface { - // Listen creates a new listening UDPConn. - Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) -} - -// ErrorWrapperQUICListener is a QUICListener that wraps errors. -type ErrorWrapperQUICListener struct { - // QUICListener is the underlying listener. - QUICListener QUICListener -} - -var _ QUICListener = &ErrorWrapperQUICListener{} - -// Listen implements QUICListener.Listen. -func (qls *ErrorWrapperQUICListener) Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) { - pconn, err := qls.QUICListener.Listen(addr) - if err != nil { - return nil, SafeErrWrapperBuilder{ - Error: err, - Operation: netxlite.QUICListenOperation, - }.MaybeBuild() - } - return &errorWrapperUDPConn{pconn}, nil -} - -// errorWrapperUDPConn is a model.UDPLikeConn that wraps errors. -type errorWrapperUDPConn struct { - // UDPLikeConn is the underlying conn. - model.UDPLikeConn -} - -var _ model.UDPLikeConn = &errorWrapperUDPConn{} - -// WriteTo implements model.UDPLikeConn.WriteTo. -func (c *errorWrapperUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) { - count, err := c.UDPLikeConn.WriteTo(p, addr) - if err != nil { - return 0, SafeErrWrapperBuilder{ - Error: err, - Operation: netxlite.WriteToOperation, - }.MaybeBuild() - } - return count, nil -} - -// ReadFrom implements model.UDPLikeConn.ReadFrom. -func (c *errorWrapperUDPConn) ReadFrom(b []byte) (int, net.Addr, error) { - n, addr, err := c.UDPLikeConn.ReadFrom(b) - if err != nil { - return 0, nil, SafeErrWrapperBuilder{ - Error: err, - Operation: netxlite.ReadFromOperation, - }.MaybeBuild() - } - return n, addr, nil -} - -// ErrorWrapperQUICDialer is a dialer that performs quic err wrapping -type ErrorWrapperQUICDialer struct { - Dialer QUICContextDialer -} - -// DialContext implements ContextDialer.DialContext -func (d *ErrorWrapperQUICDialer) DialContext( - ctx context.Context, network string, host string, - tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - sess, err := d.Dialer.DialContext(ctx, network, host, tlsCfg, cfg) - if err != nil { - return nil, SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyQUICHandshakeError, - Error: err, - Operation: netxlite.QUICHandshakeOperation, - }.MaybeBuild() - } - return sess, nil -} diff --git a/internal/engine/legacy/errorsx/quic_test.go b/internal/engine/legacy/errorsx/quic_test.go deleted file mode 100644 index d77379c..0000000 --- a/internal/engine/legacy/errorsx/quic_test.go +++ /dev/null @@ -1,157 +0,0 @@ -package errorsx - -import ( - "context" - "crypto/tls" - "errors" - "io" - "net" - "testing" - - "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/model/mocks" - "github.com/ooni/probe-cli/v3/internal/netxlite" - nlmocks "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" -) - -func TestErrorWrapperQUICListenerSuccess(t *testing.T) { - ql := &ErrorWrapperQUICListener{ - QUICListener: &mocks.QUICListener{ - MockListen: func(addr *net.UDPAddr) (model.UDPLikeConn, error) { - return &net.UDPConn{}, nil - }, - }, - } - pconn, err := ql.Listen(&net.UDPAddr{}) - if err != nil { - t.Fatal(err) - } - pconn.Close() -} - -func TestErrorWrapperQUICListenerFailure(t *testing.T) { - ql := &ErrorWrapperQUICListener{ - QUICListener: &mocks.QUICListener{ - MockListen: func(addr *net.UDPAddr) (model.UDPLikeConn, error) { - return nil, io.EOF - }, - }, - } - pconn, err := ql.Listen(&net.UDPAddr{}) - if err.Error() != "eof_error" { - t.Fatal("not the error we expected", err) - } - if pconn != nil { - t.Fatal("expected nil pconn here") - } -} - -func TestErrorWrapperUDPConnWriteToSuccess(t *testing.T) { - quc := &errorWrapperUDPConn{ - UDPLikeConn: &mocks.QUICUDPLikeConn{ - MockWriteTo: func(p []byte, addr net.Addr) (int, error) { - return 10, nil - }, - }, - } - pkt := make([]byte, 128) - addr := &net.UDPAddr{} - cnt, err := quc.WriteTo(pkt, addr) - if err != nil { - t.Fatal("not the error we expected", err) - } - if cnt != 10 { - t.Fatal("expected 10 here") - } -} - -func TestErrorWrapperUDPConnWriteToFailure(t *testing.T) { - expected := errors.New("mocked error") - quc := &errorWrapperUDPConn{ - UDPLikeConn: &mocks.QUICUDPLikeConn{ - MockWriteTo: func(p []byte, addr net.Addr) (int, error) { - return 0, expected - }, - }, - } - pkt := make([]byte, 128) - addr := &net.UDPAddr{} - cnt, err := quc.WriteTo(pkt, addr) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected", err) - } - if cnt != 0 { - t.Fatal("expected 0 here") - } -} - -func TestErrorWrapperUDPConnReadFromSuccess(t *testing.T) { - expected := errors.New("mocked error") - quc := &errorWrapperUDPConn{ - UDPLikeConn: &mocks.QUICUDPLikeConn{ - MockReadFrom: func(b []byte) (int, net.Addr, error) { - return 0, nil, expected - }, - }, - } - b := make([]byte, 128) - n, addr, err := quc.ReadFrom(b) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected", err) - } - if n != 0 { - t.Fatal("expected 0 here") - } - if addr != nil { - t.Fatal("expected nil here") - } -} - -func TestErrorWrapperUDPConnReadFromFailure(t *testing.T) { - quc := &errorWrapperUDPConn{ - UDPLikeConn: &mocks.QUICUDPLikeConn{ - MockReadFrom: func(b []byte) (int, net.Addr, error) { - return 10, nil, nil - }, - }, - } - b := make([]byte, 128) - n, addr, err := quc.ReadFrom(b) - if err != nil { - t.Fatal("not the error we expected", err) - } - if n != 10 { - t.Fatal("expected 10 here") - } - if addr != nil { - t.Fatal("expected nil here") - } -} - -func TestErrorWrapperQUICDialerFailure(t *testing.T) { - ctx := context.Background() - d := &ErrorWrapperQUICDialer{Dialer: &nlmocks.QUICContextDialer{ - MockDialContext: func(ctx context.Context, network, address string, tlsConfig *tls.Config, quicConfig *quic.Config) (quic.EarlySession, error) { - return nil, io.EOF - }, - }} - sess, err := d.DialContext( - ctx, "udp", "www.google.com:443", &tls.Config{}, &quic.Config{}) - if sess != nil { - t.Fatal("expected a nil sess here") - } - if !errors.Is(err, io.EOF) { - t.Fatal("expected another error here") - } - var errWrapper *netxlite.ErrWrapper - if !errors.As(err, &errWrapper) { - t.Fatal("cannot cast to ErrWrapper") - } - if errWrapper.Operation != netxlite.QUICHandshakeOperation { - t.Fatal("unexpected Operation") - } - if errWrapper.Failure != netxlite.FailureEOFError { - t.Fatal("unexpected failure") - } -} diff --git a/internal/engine/legacy/errorsx/resolver.go b/internal/engine/legacy/errorsx/resolver.go deleted file mode 100644 index 90c8e28..0000000 --- a/internal/engine/legacy/errorsx/resolver.go +++ /dev/null @@ -1,56 +0,0 @@ -package errorsx - -import ( - "context" - - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// 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: netxlite.ClassifyResolverError, - Error: err, - Operation: netxlite.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/engine/legacy/errorsx/resolver_test.go b/internal/engine/legacy/errorsx/resolver_test.go deleted file mode 100644 index f04825a..0000000 --- a/internal/engine/legacy/errorsx/resolver_test.go +++ /dev/null @@ -1,81 +0,0 @@ -package errorsx - -import ( - "context" - "errors" - "net" - "testing" - - "github.com/ooni/probe-cli/v3/internal/model/mocks" - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -func TestErrorWrapperResolverSuccess(t *testing.T) { - orig := []string{"8.8.8.8"} - r := &ErrorWrapperResolver{ - Resolver: &mocks.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: &mocks.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 *netxlite.ErrWrapper - if !errors.As(err, &errWrapper) { - t.Fatal("cannot properly cast the returned error") - } - if errWrapper.Failure != netxlite.FailureDNSNXDOMAINError { - t.Fatal("unexpected failure") - } - if errWrapper.Operation != netxlite.ResolveOperation { - t.Fatal("unexpected Operation") - } -} - -func TestErrorWrapperResolverChildNetworkAddress(t *testing.T) { - r := &ErrorWrapperResolver{Resolver: &mocks.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") - } -} diff --git a/internal/engine/legacy/errorsx/tls.go b/internal/engine/legacy/errorsx/tls.go deleted file mode 100644 index 7fb251c..0000000 --- a/internal/engine/legacy/errorsx/tls.go +++ /dev/null @@ -1,33 +0,0 @@ -package errorsx - -import ( - "context" - "crypto/tls" - "net" - - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -// TLSHandshaker is the generic TLS handshaker -type TLSHandshaker interface { - Handshake(ctx context.Context, conn net.Conn, config *tls.Config) ( - net.Conn, tls.ConnectionState, error) -} - -// ErrorWrapperTLSHandshaker wraps the returned error to be an OONI error -type ErrorWrapperTLSHandshaker struct { - TLSHandshaker -} - -// Handshake implements TLSHandshaker.Handshake -func (h *ErrorWrapperTLSHandshaker) Handshake( - ctx context.Context, conn net.Conn, config *tls.Config, -) (net.Conn, tls.ConnectionState, error) { - tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) - err = SafeErrWrapperBuilder{ - Classifier: netxlite.ClassifyTLSHandshakeError, - Error: err, - Operation: netxlite.TLSHandshakeOperation, - }.MaybeBuild() - return tlsconn, state, err -} diff --git a/internal/engine/legacy/errorsx/tls_test.go b/internal/engine/legacy/errorsx/tls_test.go deleted file mode 100644 index da91256..0000000 --- a/internal/engine/legacy/errorsx/tls_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package errorsx - -import ( - "context" - "crypto/tls" - "errors" - "io" - "net" - "testing" - - "github.com/ooni/probe-cli/v3/internal/model/mocks" - "github.com/ooni/probe-cli/v3/internal/netxlite" -) - -func TestErrorWrapperTLSHandshakerFailure(t *testing.T) { - th := ErrorWrapperTLSHandshaker{TLSHandshaker: &mocks.TLSHandshaker{ - MockHandshake: func(ctx context.Context, conn net.Conn, config *tls.Config) (net.Conn, tls.ConnectionState, error) { - return nil, tls.ConnectionState{}, io.EOF - }, - }} - conn, _, err := th.Handshake( - context.Background(), &mocks.Conn{ - MockRead: func(b []byte) (int, error) { - return 0, io.EOF - }, - }, new(tls.Config)) - if !errors.Is(err, io.EOF) { - t.Fatal("not the error that we expected") - } - if conn != nil { - t.Fatal("expected nil con here") - } - var errWrapper *netxlite.ErrWrapper - if !errors.As(err, &errWrapper) { - t.Fatal("cannot cast to ErrWrapper") - } - if errWrapper.Failure != netxlite.FailureEOFError { - t.Fatal("unexpected Failure") - } - if errWrapper.Operation != netxlite.TLSHandshakeOperation { - t.Fatal("unexpected Operation") - } -} diff --git a/internal/engine/netx/archival/archival.go b/internal/engine/netx/archival/archival.go index f930768..3655b5e 100644 --- a/internal/engine/netx/archival/archival.go +++ b/internal/engine/netx/archival/archival.go @@ -17,7 +17,6 @@ import ( "unicode/utf8" "github.com/ooni/probe-cli/v3/internal/engine/geolocate" - errorsxlegacy "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" @@ -111,10 +110,7 @@ func NewFailure(err error) *string { // The following code guarantees that the error is always wrapped even // when we could not actually hit our code that does the wrapping. A case // in which this happen is with context deadline for HTTP. - err = errorsxlegacy.SafeErrWrapperBuilder{ - Error: err, - Operation: netxlite.TopLevelOperation, - }.MaybeBuild() + err = netxlite.NewTopLevelGenericErrWrapper(err) errWrapper := err.(*netxlite.ErrWrapper) s := errWrapper.Failure if s == "" { diff --git a/internal/engine/netx/dialer/dialer.go b/internal/engine/netx/dialer/dialer.go index 572dc0d..e704009 100644 --- a/internal/engine/netx/dialer/dialer.go +++ b/internal/engine/netx/dialer/dialer.go @@ -5,7 +5,6 @@ import ( "net" "net/url" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" @@ -61,8 +60,7 @@ type Config struct { // New creates a new Dialer from the specified config and resolver. func New(config *Config, resolver Resolver) Dialer { - var d Dialer = netxlite.DefaultDialer - d = &errorsx.ErrorWrapperDialer{Dialer: d} + var d Dialer = &netxlite.ErrorWrapperDialer{Dialer: netxlite.DefaultDialer} if config.Logger != nil { d = &netxlite.DialerLogger{ Dialer: netxlite.NewDialerLegacyAdapter(d), diff --git a/internal/engine/netx/dialer/dialer_test.go b/internal/engine/netx/dialer/dialer_test.go index 15fd200..eab23f1 100644 --- a/internal/engine/netx/dialer/dialer_test.go +++ b/internal/engine/netx/dialer/dialer_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -56,7 +55,7 @@ func TestNewCreatesTheExpectedChain(t *testing.T) { if !ok { t.Fatal("invalid type") } - ewd, ok := dad.DialerLegacy.(*errorsx.ErrorWrapperDialer) + ewd, ok := dad.DialerLegacy.(*netxlite.ErrorWrapperDialer) if !ok { t.Fatal("not an errorWrappingDialer") } diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index f120a84..180820f 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -32,7 +32,6 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" @@ -129,7 +128,7 @@ func NewResolver(config Config) Resolver { if config.BogonIsError { r = resolver.BogonResolver{Resolver: r} } - r = &errorsx.ErrorWrapperResolver{Resolver: r} + r = &netxlite.ErrorWrapperResolver{Resolver: netxlite.NewResolverLegacyAdapter(r)} if config.Logger != nil { r = &netxlite.ResolverLogger{ Logger: config.Logger, @@ -162,7 +161,7 @@ func NewQUICDialer(config Config) QUICDialer { config.FullResolver = NewResolver(config) } var ql quicdialer.QUICListener = &netxlite.QUICListenerStdlib{} - ql = &errorsx.ErrorWrapperQUICListener{QUICListener: ql} + ql = &netxlite.ErrorWrapperQUICListener{QUICListener: ql} if config.ReadWriteSaver != nil { ql = &quicdialer.QUICListenerSaver{ QUICListener: ql, @@ -172,7 +171,9 @@ func NewQUICDialer(config Config) QUICDialer { var d quicdialer.ContextDialer = &netxlite.QUICDialerQUICGo{ QUICListener: ql, } - d = &errorsx.ErrorWrapperQUICDialer{Dialer: d} + d = &netxlite.ErrorWrapperQUICDialer{ + QUICDialer: netxlite.NewQUICDialerFromContextDialerAdapter(d), + } if config.TLSSaver != nil { d = quicdialer.HandshakeSaver{Saver: config.TLSSaver, Dialer: d} } @@ -189,7 +190,7 @@ func NewTLSDialer(config Config) TLSDialer { config.Dialer = NewDialer(config) } var h tlsHandshaker = &netxlite.TLSHandshakerConfigurable{} - h = &errorsx.ErrorWrapperTLSHandshaker{TLSHandshaker: h} + h = &netxlite.ErrorWrapperTLSHandshaker{TLSHandshaker: h} if config.Logger != nil { h = &netxlite.TLSHandshakerLogger{DebugLogger: config.Logger, TLSHandshaker: h} } diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 093a0c5..c5e3f90 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -11,7 +11,6 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/bytecounter" - "github.com/ooni/probe-cli/v3/internal/engine/legacy/errorsx" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" @@ -31,11 +30,11 @@ func TestNewResolverVanilla(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*resolver.AddressResolver) + ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -63,11 +62,11 @@ func TestNewResolverSpecificResolver(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*resolver.AddressResolver) + ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -93,11 +92,11 @@ func TestNewResolverWithBogonFilter(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - br, ok := ewr.Resolver.(resolver.BogonResolver) + br, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(resolver.BogonResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -146,11 +145,11 @@ func TestNewResolverWithLogging(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatalf("not the resolver we expected %T", rla.ResolverLegacy) } - ar, ok := ewr.Resolver.(*resolver.AddressResolver) + ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -184,11 +183,11 @@ func TestNewResolverWithSaver(t *testing.T) { if sr.Saver != saver { t.Fatal("not the saver we expected") } - ewr, ok := sr.Resolver.(*errorsx.ErrorWrapperResolver) + ewr, ok := sr.Resolver.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - ar, ok := ewr.Resolver.(*resolver.AddressResolver) + ar, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.AddressResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -214,11 +213,11 @@ func TestNewResolverWithReadWriteCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - cr, ok := ewr.Resolver.(*resolver.CacheResolver) + cr, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.CacheResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -253,11 +252,11 @@ func TestNewResolverWithPrefilledReadonlyCache(t *testing.T) { if !ok { t.Fatal("not the resolver we expected") } - ewr, ok := rla.ResolverLegacy.(*errorsx.ErrorWrapperResolver) + ewr, ok := rla.ResolverLegacy.(*netxlite.ErrorWrapperResolver) if !ok { t.Fatal("not the resolver we expected") } - cr, ok := ewr.Resolver.(*resolver.CacheResolver) + cr, ok := ewr.Resolver.(*netxlite.ResolverLegacyAdapter).ResolverLegacy.(*resolver.CacheResolver) if !ok { t.Fatal("not the resolver we expected") } @@ -302,7 +301,7 @@ func TestNewTLSDialerVanilla(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -331,7 +330,7 @@ func TestNewTLSDialerWithConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -370,7 +369,7 @@ func TestNewTLSDialerWithLogging(t *testing.T) { if lth.DebugLogger != log.Log { t.Fatal("not the Logger we expected") } - ewth, ok := lth.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := lth.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -410,7 +409,7 @@ func TestNewTLSDialerWithSaver(t *testing.T) { if sth.Saver != saver { t.Fatal("not the Logger we expected") } - ewth, ok := sth.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := sth.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -443,7 +442,7 @@ func TestNewTLSDialerWithNoTLSVerifyAndConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } @@ -478,7 +477,7 @@ func TestNewTLSDialerWithNoTLSVerifyAndNoConfig(t *testing.T) { if rtd.TLSHandshaker == nil { t.Fatal("invalid TLSHandshaker") } - ewth, ok := rtd.TLSHandshaker.(*errorsx.ErrorWrapperTLSHandshaker) + ewth, ok := rtd.TLSHandshaker.(*netxlite.ErrorWrapperTLSHandshaker) if !ok { t.Fatal("not the TLSHandshaker we expected") } diff --git a/internal/netxlite/classify.go b/internal/netxlite/classify.go index 77dae38..4e0b1b2 100644 --- a/internal/netxlite/classify.go +++ b/internal/netxlite/classify.go @@ -11,7 +11,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/scrubber" ) -// ClassifyGenericError is maps an error occurred during an operation +// classifyGenericError is maps an error occurred during an operation // to an OONI failure string. This specific classifier is the most // generic one. You usually use it when mapping I/O errors. You should // check whether there is a specific classifier for more specific @@ -34,7 +34,7 @@ import ( // If everything else fails, this classifier returns a string // like "unknown_failure: XXX" where XXX has been scrubbed // so to remove any network endpoints from the original error string. -func ClassifyGenericError(err error) string { +func classifyGenericError(err error) string { // The list returned here matches the values used by MK unless // explicitly noted otherwise with a comment. @@ -136,7 +136,7 @@ const ( quicTLSUnrecognizedName = 112 ) -// ClassifyQUICHandshakeError maps errors during a QUIC +// classifyQUICHandshakeError maps errors during a QUIC // handshake to OONI failure strings. // // If the input error is an *ErrWrapper we don't perform @@ -144,7 +144,7 @@ const ( // // If this classifier fails, it calls ClassifyGenericError // and returns to the caller its return value. -func ClassifyQUICHandshakeError(err error) string { +func classifyQUICHandshakeError(err error) string { // QUIRK: we cannot remove this check as long as this function // is exported and used independently from NewErrWrapper. @@ -205,7 +205,7 @@ func ClassifyQUICHandshakeError(err error) string { } } } - return ClassifyGenericError(err) + return classifyGenericError(err) } // quicIsCertificateError tells us whether a specific TLS alert error @@ -253,7 +253,7 @@ var ( ErrOODNSNoAnswer = fmt.Errorf("ooniresolver: %s", DNSNoAnswerSuffix) ) -// ClassifyResolverError maps DNS resolution errors to +// classifyResolverError maps DNS resolution errors to // OONI failure strings. // // If the input error is an *ErrWrapper we don't perform @@ -261,7 +261,7 @@ var ( // // If this classifier fails, it calls ClassifyGenericError and // returns to the caller its return value. -func ClassifyResolverError(err error) string { +func classifyResolverError(err error) string { // QUIRK: we cannot remove this check as long as this function // is exported and used independently from NewErrWrapper. @@ -278,10 +278,10 @@ func ClassifyResolverError(err error) string { if errors.Is(err, ErrOODNSRefused) { return FailureDNSRefusedError // not in MK } - return ClassifyGenericError(err) + return classifyGenericError(err) } -// ClassifyTLSHandshakeError maps an error occurred during the TLS +// classifyTLSHandshakeError maps an error occurred during the TLS // handshake to an OONI failure string. // // If the input error is an *ErrWrapper we don't perform @@ -289,7 +289,7 @@ func ClassifyResolverError(err error) string { // // If this classifier fails, it calls ClassifyGenericError and // returns to the caller its return value. -func ClassifyTLSHandshakeError(err error) string { +func classifyTLSHandshakeError(err error) string { // QUIRK: we cannot remove this check as long as this function // is exported and used independently from NewErrWrapper. @@ -314,5 +314,5 @@ func ClassifyTLSHandshakeError(err error) string { // Test case: https://expired.badssl.com/ return FailureSSLInvalidCertificate } - return ClassifyGenericError(err) + return classifyGenericError(err) } diff --git a/internal/netxlite/classify_test.go b/internal/netxlite/classify_test.go index 21f05b5..22c4949 100644 --- a/internal/netxlite/classify_test.go +++ b/internal/netxlite/classify_test.go @@ -18,13 +18,13 @@ func TestClassifyGenericError(t *testing.T) { t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} - if ClassifyGenericError(err) != FailureEOFError { + if classifyGenericError(err) != FailureEOFError { t.Fatal("did not classify existing ErrWrapper correctly") } }) t.Run("for a system call error", func(t *testing.T) { - if ClassifyGenericError(EWOULDBLOCK) != FailureOperationWouldBlock { + if classifyGenericError(EWOULDBLOCK) != FailureOperationWouldBlock { t.Fatal("unexpected results") } }) @@ -35,63 +35,63 @@ func TestClassifyGenericError(t *testing.T) { // is just an implementation detail. t.Run("for operation was canceled", func(t *testing.T) { - if ClassifyGenericError(errors.New("operation was canceled")) != FailureInterrupted { + if classifyGenericError(errors.New("operation was canceled")) != FailureInterrupted { t.Fatal("unexpected result") } }) t.Run("for EOF", func(t *testing.T) { - if ClassifyGenericError(io.EOF) != FailureEOFError { + if classifyGenericError(io.EOF) != FailureEOFError { t.Fatal("unexpected result") } }) t.Run("for context deadline exceeded", func(t *testing.T) { - if ClassifyGenericError(context.DeadlineExceeded) != FailureGenericTimeoutError { + if classifyGenericError(context.DeadlineExceeded) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for stun's transaction is timed out", func(t *testing.T) { - if ClassifyGenericError(stun.ErrTransactionTimeOut) != FailureGenericTimeoutError { + if classifyGenericError(stun.ErrTransactionTimeOut) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for i/o timeout", func(t *testing.T) { - if ClassifyGenericError(errors.New("i/o timeout")) != FailureGenericTimeoutError { + if classifyGenericError(errors.New("i/o timeout")) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for TLS handshake timeout", func(t *testing.T) { err := errors.New("net/http: TLS handshake timeout") - if ClassifyGenericError(err) != FailureGenericTimeoutError { + if classifyGenericError(err) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for no such host", func(t *testing.T) { - if ClassifyGenericError(errors.New("no such host")) != FailureDNSNXDOMAINError { + if classifyGenericError(errors.New("no such host")) != FailureDNSNXDOMAINError { t.Fatal("unexpected results") } }) t.Run("for dns server misbehaving", func(t *testing.T) { - if ClassifyGenericError(errors.New("dns server misbehaving")) != FailureDNSServerMisbehaving { + if classifyGenericError(errors.New("dns server misbehaving")) != FailureDNSServerMisbehaving { t.Fatal("unexpected results") } }) t.Run("for no answer from DNS server", func(t *testing.T) { - if ClassifyGenericError(errors.New("no answer from DNS server")) != FailureDNSNoAnswer { + if classifyGenericError(errors.New("no answer from DNS server")) != FailureDNSNoAnswer { t.Fatal("unexpected results") } }) t.Run("for use of closed network connection", func(t *testing.T) { err := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: use of closed network connection") - if ClassifyGenericError(err) != FailureConnectionAlreadyClosed { + if classifyGenericError(err) != FailureConnectionAlreadyClosed { t.Fatal("unexpected results") } }) @@ -99,7 +99,7 @@ func TestClassifyGenericError(t *testing.T) { // Now we're back in ClassifyGenericError t.Run("for context.Canceled", func(t *testing.T) { - if ClassifyGenericError(context.Canceled) != FailureInterrupted { + if classifyGenericError(context.Canceled) != FailureInterrupted { t.Fatal("unexpected result") } }) @@ -108,7 +108,7 @@ func TestClassifyGenericError(t *testing.T) { t.Run("with an IPv4 address", func(t *testing.T) { input := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: some error") expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: some error" - out := ClassifyGenericError(input) + out := classifyGenericError(input) if out != expected { t.Fatal(cmp.Diff(expected, out)) } @@ -117,7 +117,7 @@ func TestClassifyGenericError(t *testing.T) { t.Run("with an IPv6 address", func(t *testing.T) { input := errors.New("read tcp [::1]:56948->[::1]:443: some error") expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: some error" - out := ClassifyGenericError(input) + out := classifyGenericError(input) if out != expected { t.Fatal(cmp.Diff(expected, out)) } @@ -131,100 +131,100 @@ func TestClassifyQUICHandshakeError(t *testing.T) { t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} - if ClassifyQUICHandshakeError(err) != FailureEOFError { + if classifyQUICHandshakeError(err) != FailureEOFError { t.Fatal("did not classify existing ErrWrapper correctly") } }) t.Run("for incompatible quic version", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.VersionNegotiationError{}) != FailureQUICIncompatibleVersion { + if classifyQUICHandshakeError(&quic.VersionNegotiationError{}) != FailureQUICIncompatibleVersion { t.Fatal("unexpected results") } }) t.Run("for stateless reset", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset { + if classifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset { t.Fatal("unexpected results") } }) t.Run("for handshake timeout", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.HandshakeTimeoutError{}) != FailureGenericTimeoutError { + if classifyQUICHandshakeError(&quic.HandshakeTimeoutError{}) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for idle timeout", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.IdleTimeoutError{}) != FailureGenericTimeoutError { + if classifyQUICHandshakeError(&quic.IdleTimeoutError{}) != FailureGenericTimeoutError { t.Fatal("unexpected results") } }) t.Run("for connection refused", func(t *testing.T) { - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused { t.Fatal("unexpected results") } }) t.Run("for bad certificate", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertBadCertificate - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) t.Run("for unsupported certificate", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertUnsupportedCertificate - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) t.Run("for certificate expired", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertCertificateExpired - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) t.Run("for certificate revoked", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertCertificateRevoked - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) t.Run("for certificate unknown", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertCertificateUnknown - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate { t.Fatal("unexpected results") } }) t.Run("for decrypt error", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertDecryptError - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { t.Fatal("unexpected results") } }) t.Run("for handshake failure", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake { t.Fatal("unexpected results") } }) t.Run("for unknown CA", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSAlertUnknownCA - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLUnknownAuthority { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLUnknownAuthority { t.Fatal("unexpected results") } }) t.Run("for unrecognized hostname", func(t *testing.T) { var err quic.TransportErrorCode = quicTLSUnrecognizedName - if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidHostname { + if classifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidHostname { t.Fatal("unexpected results") } }) @@ -234,13 +234,13 @@ func TestClassifyQUICHandshakeError(t *testing.T) { ErrorCode: quic.InternalError, ErrorMessage: FailureHostUnreachable, } - if ClassifyQUICHandshakeError(err) != FailureHostUnreachable { + if classifyQUICHandshakeError(err) != FailureHostUnreachable { t.Fatal("unexpected results") } }) t.Run("for another kind of error", func(t *testing.T) { - if ClassifyQUICHandshakeError(io.EOF) != FailureEOFError { + if classifyQUICHandshakeError(io.EOF) != FailureEOFError { t.Fatal("unexpected result") } }) @@ -252,25 +252,25 @@ func TestClassifyResolverError(t *testing.T) { t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} - if ClassifyResolverError(err) != FailureEOFError { + if classifyResolverError(err) != FailureEOFError { t.Fatal("did not classify existing ErrWrapper correctly") } }) t.Run("for ErrDNSBogon", func(t *testing.T) { - if ClassifyResolverError(ErrDNSBogon) != FailureDNSBogonError { + if classifyResolverError(ErrDNSBogon) != FailureDNSBogonError { t.Fatal("unexpected result") } }) t.Run("for refused", func(t *testing.T) { - if ClassifyResolverError(ErrOODNSRefused) != FailureDNSRefusedError { + if classifyResolverError(ErrOODNSRefused) != FailureDNSRefusedError { t.Fatal("unexpected result") } }) t.Run("for another kind of error", func(t *testing.T) { - if ClassifyResolverError(io.EOF) != FailureEOFError { + if classifyResolverError(io.EOF) != FailureEOFError { t.Fatal("unexpected result") } }) @@ -282,34 +282,34 @@ func TestClassifyTLSHandshakeError(t *testing.T) { t.Run("for input being already an ErrWrapper", func(t *testing.T) { err := &ErrWrapper{Failure: FailureEOFError} - if ClassifyTLSHandshakeError(err) != FailureEOFError { + if classifyTLSHandshakeError(err) != FailureEOFError { t.Fatal("did not classify existing ErrWrapper correctly") } }) t.Run("for x509.HostnameError", func(t *testing.T) { var err x509.HostnameError - if ClassifyTLSHandshakeError(err) != FailureSSLInvalidHostname { + if classifyTLSHandshakeError(err) != FailureSSLInvalidHostname { t.Fatal("unexpected result") } }) t.Run("for x509.UnknownAuthorityError", func(t *testing.T) { var err x509.UnknownAuthorityError - if ClassifyTLSHandshakeError(err) != FailureSSLUnknownAuthority { + if classifyTLSHandshakeError(err) != FailureSSLUnknownAuthority { t.Fatal("unexpected result") } }) t.Run("for x509.CertificateInvalidError", func(t *testing.T) { var err x509.CertificateInvalidError - if ClassifyTLSHandshakeError(err) != FailureSSLInvalidCertificate { + if classifyTLSHandshakeError(err) != FailureSSLInvalidCertificate { t.Fatal("unexpected result") } }) t.Run("for another kind of error", func(t *testing.T) { - if ClassifyTLSHandshakeError(io.EOF) != FailureEOFError { + if classifyTLSHandshakeError(io.EOF) != FailureEOFError { t.Fatal("unexpected result") } }) diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go index 07eb2d0..4c1f754 100644 --- a/internal/netxlite/dialer.go +++ b/internal/netxlite/dialer.go @@ -226,7 +226,7 @@ var _ model.Dialer = &dialerErrWrapper{} func (d *dialerErrWrapper) DialContext(ctx context.Context, network, address string) (net.Conn, error) { conn, err := d.Dialer.DialContext(ctx, network, address) if err != nil { - return nil, NewErrWrapper(ClassifyGenericError, ConnectOperation, err) + return nil, NewErrWrapper(classifyGenericError, ConnectOperation, err) } return &dialerErrWrapperConn{Conn: conn}, nil } @@ -241,7 +241,7 @@ var _ net.Conn = &dialerErrWrapperConn{} func (c *dialerErrWrapperConn) Read(b []byte) (int, error) { count, err := c.Conn.Read(b) if err != nil { - return 0, NewErrWrapper(ClassifyGenericError, ReadOperation, err) + return 0, NewErrWrapper(classifyGenericError, ReadOperation, err) } return count, nil } @@ -249,7 +249,7 @@ func (c *dialerErrWrapperConn) Read(b []byte) (int, error) { func (c *dialerErrWrapperConn) Write(b []byte) (int, error) { count, err := c.Conn.Write(b) if err != nil { - return 0, NewErrWrapper(ClassifyGenericError, WriteOperation, err) + return 0, NewErrWrapper(classifyGenericError, WriteOperation, err) } return count, nil } @@ -257,7 +257,7 @@ func (c *dialerErrWrapperConn) Write(b []byte) (int, error) { func (c *dialerErrWrapperConn) Close() error { err := c.Conn.Close() if err != nil { - return NewErrWrapper(ClassifyGenericError, CloseOperation, err) + return NewErrWrapper(classifyGenericError, CloseOperation, err) } return nil } diff --git a/internal/netxlite/dialer_test.go b/internal/netxlite/dialer_test.go index 2031046..4496d07 100644 --- a/internal/netxlite/dialer_test.go +++ b/internal/netxlite/dialer_test.go @@ -254,7 +254,7 @@ func TestDialerResolver(t *testing.T) { errorsList := []error{ errors.New("a mocked error"), NewErrWrapper( - ClassifyGenericError, + classifyGenericError, CloseOperation, io.EOF, ), @@ -296,7 +296,7 @@ func TestDialerResolver(t *testing.T) { errorsList := []error{ errors.New("a mocked error"), NewErrWrapper( - ClassifyGenericError, + classifyGenericError, CloseOperation, errors.New("antani"), ), diff --git a/internal/netxlite/errwrapper.go b/internal/netxlite/errwrapper.go index c618399..121ea2a 100644 --- a/internal/netxlite/errwrapper.go +++ b/internal/netxlite/errwrapper.go @@ -22,18 +22,13 @@ type ErrWrapper struct { // Operation is the operation that failed. // - // New code will always nest ErrWrapper and you need to - // walk the chain to find what happened. - // - // The following comment describes the DEPRECATED - // legacy behavior implements by internal/engine/legacy/errorsx: - // - // If possible, the Operation string - // SHOULD be a _major_ operation. Major operations are: + // If possible, the Operation string SHOULD be a _major_ + // operation. Major operations are: // // - ResolveOperation: resolving a domain name failed // - ConnectOperation: connecting to an IP failed // - TLSHandshakeOperation: TLS handshaking failed + // - QUICHandshakeOperation: QUIC handshaking failed // - HTTPRoundTripOperation: other errors during round trip // // Because a network connection doesn't necessarily know @@ -84,13 +79,14 @@ type Classifier func(err error) string // // If the err argument has already been classified, the returned // error wrapper will use the same classification string and -// failed operation of the original wrapped error. +// will determine whether to keep the major operation as documented +// in the ErrWrapper.Operation documentation. func NewErrWrapper(c Classifier, op string, err error) *ErrWrapper { var wrapper *ErrWrapper if errors.As(err, &wrapper) { return &ErrWrapper{ Failure: wrapper.Failure, - Operation: wrapper.Operation, + Operation: classifyOperation(wrapper, op), WrappedErr: err, } } @@ -117,5 +113,37 @@ func NewErrWrapper(c Classifier, op string, err error) *ErrWrapper { // error wrapper will use the same classification string and // failed operation of the original error. func NewTopLevelGenericErrWrapper(err error) *ErrWrapper { - return NewErrWrapper(ClassifyGenericError, TopLevelOperation, err) + return NewErrWrapper(classifyGenericError, TopLevelOperation, err) +} + +func classifyOperation(ew *ErrWrapper, operation string) string { + // Basically, as explained in ErrWrapper docs, let's + // keep the child major operation, if any. + // + // QUIRK: this code is legacy code and we should not change + // it unless we also change the experiments that depend on it + // for determining the blocking reason based on the failed + // operation value (e.g., telegram, web connectivity). + if ew.Operation == ConnectOperation { + return ew.Operation + } + if ew.Operation == HTTPRoundTripOperation { + return ew.Operation + } + if ew.Operation == ResolveOperation { + return ew.Operation + } + if ew.Operation == TLSHandshakeOperation { + return ew.Operation + } + if ew.Operation == QUICHandshakeOperation { + return ew.Operation + } + if ew.Operation == "quic_handshake_start" { + return QUICHandshakeOperation + } + if ew.Operation == "quic_handshake_done" { + return QUICHandshakeOperation + } + return operation } diff --git a/internal/netxlite/errwrapper_test.go b/internal/netxlite/errwrapper_test.go index 97cc645..e4066d3 100644 --- a/internal/netxlite/errwrapper_test.go +++ b/internal/netxlite/errwrapper_test.go @@ -68,7 +68,7 @@ func TestNewErrWrapper(t *testing.T) { recovered.Add(1) } }() - NewErrWrapper(ClassifyGenericError, "", io.EOF) + NewErrWrapper(classifyGenericError, "", io.EOF) }() if recovered.Load() != 1 { t.Fatal("did not panic") @@ -83,7 +83,7 @@ func TestNewErrWrapper(t *testing.T) { recovered.Add(1) } }() - NewErrWrapper(ClassifyGenericError, CloseOperation, nil) + NewErrWrapper(classifyGenericError, CloseOperation, nil) }() if recovered.Load() != 1 { t.Fatal("did not panic") @@ -91,7 +91,7 @@ func TestNewErrWrapper(t *testing.T) { }) t.Run("otherwise, works as intended", func(t *testing.T) { - ew := NewErrWrapper(ClassifyGenericError, CloseOperation, io.EOF) + ew := NewErrWrapper(classifyGenericError, CloseOperation, io.EOF) if ew.Failure != FailureEOFError { t.Fatal("unexpected failure") } @@ -107,11 +107,11 @@ func TestNewErrWrapper(t *testing.T) { ew := NewErrWrapper(classifySyscallError, ReadOperation, ECONNRESET) var err1 error = ew err2 := fmt.Errorf("cannot read: %w", err1) - ew2 := NewErrWrapper(ClassifyGenericError, TopLevelOperation, err2) + ew2 := NewErrWrapper(classifyGenericError, HTTPRoundTripOperation, err2) if ew2.Failure != ew.Failure { t.Fatal("not the same failure") } - if ew2.Operation != ew.Operation { + if ew2.Operation != HTTPRoundTripOperation { t.Fatal("not the same operation") } if ew2.WrappedErr != err2 { @@ -135,3 +135,78 @@ func TestNewTopLevelGenericErrWrapper(t *testing.T) { t.Fatal("invalid WrappedErr") } } + +func TestClassifyOperation(t *testing.T) { + t.Run("for connect", func(t *testing.T) { + // You're doing HTTP and connect fails. You want to know + // that connect failed not that HTTP failed. + err := &ErrWrapper{Operation: ConnectOperation} + if classifyOperation(err, HTTPRoundTripOperation) != ConnectOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for http_round_trip", func(t *testing.T) { + // You're doing DoH and something fails inside HTTP. You want + // to know about the internal HTTP error, not resolve. + err := &ErrWrapper{Operation: HTTPRoundTripOperation} + if classifyOperation(err, ResolveOperation) != HTTPRoundTripOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for resolve", func(t *testing.T) { + // You're doing HTTP and the DNS fails. You want to + // know that resolve failed. + err := &ErrWrapper{Operation: ResolveOperation} + if classifyOperation(err, HTTPRoundTripOperation) != ResolveOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for tls_handshake", func(t *testing.T) { + // You're doing HTTP and the TLS handshake fails. You want + // to know about a TLS handshake error. + err := &ErrWrapper{Operation: TLSHandshakeOperation} + if classifyOperation(err, HTTPRoundTripOperation) != TLSHandshakeOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for minor operation", func(t *testing.T) { + // You just noticed that TLS handshake failed and you + // have a child error telling you that read failed. Here + // you want to know about a TLS handshake error. + err := &ErrWrapper{Operation: ReadOperation} + if classifyOperation(err, TLSHandshakeOperation) != TLSHandshakeOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for quic_handshake", func(t *testing.T) { + // You're doing HTTP and the QUIC handshake fails. You want + // to know about a QUIC handshake error. + err := &ErrWrapper{Operation: QUICHandshakeOperation} + if classifyOperation(err, HTTPRoundTripOperation) != QUICHandshakeOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for quic_handshake_start", func(t *testing.T) { + // You're doing HTTP and the QUIC handshake fails. You want + // to know about a QUIC handshake error. + err := &ErrWrapper{Operation: "quic_handshake_start"} + if classifyOperation(err, HTTPRoundTripOperation) != QUICHandshakeOperation { + t.Fatal("unexpected result") + } + }) + + t.Run("for quic_handshake_done", func(t *testing.T) { + // You're doing HTTP and the QUIC handshake fails. You want + // to know about a QUIC handshake error. + err := &ErrWrapper{Operation: "quic_handshake_done"} + if classifyOperation(err, HTTPRoundTripOperation) != QUICHandshakeOperation { + t.Fatal("unexpected result") + } + }) +} diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go index 9f79f61..515795f 100644 --- a/internal/netxlite/legacy.go +++ b/internal/netxlite/legacy.go @@ -26,6 +26,11 @@ type ( DialerResolver = dialerResolver DialerLogger = dialerLogger HTTPTransportLogger = httpTransportLogger + ErrorWrapperDialer = dialerErrWrapper + ErrorWrapperQUICListener = quicListenerErrWrapper + ErrorWrapperQUICDialer = quicDialerErrWrapper + ErrorWrapperResolver = resolverErrWrapper + ErrorWrapperTLSHandshaker = tlsHandshakerErrWrapper QUICListenerStdlib = quicListenerStdlib QUICDialerQUICGo = quicDialerQUICGo QUICDialerResolver = quicDialerResolver diff --git a/internal/netxlite/quic.go b/internal/netxlite/quic.go index 14aea9a..066a86d 100644 --- a/internal/netxlite/quic.go +++ b/internal/netxlite/quic.go @@ -333,7 +333,7 @@ var _ model.QUICListener = &quicListenerErrWrapper{} func (qls *quicListenerErrWrapper) Listen(addr *net.UDPAddr) (model.UDPLikeConn, error) { pconn, err := qls.QUICListener.Listen(addr) if err != nil { - return nil, NewErrWrapper(ClassifyGenericError, QUICListenOperation, err) + return nil, NewErrWrapper(classifyGenericError, QUICListenOperation, err) } return &quicErrWrapperUDPLikeConn{pconn}, nil } @@ -350,7 +350,7 @@ var _ model.UDPLikeConn = &quicErrWrapperUDPLikeConn{} func (c *quicErrWrapperUDPLikeConn) WriteTo(p []byte, addr net.Addr) (int, error) { count, err := c.UDPLikeConn.WriteTo(p, addr) if err != nil { - return 0, NewErrWrapper(ClassifyGenericError, WriteToOperation, err) + return 0, NewErrWrapper(classifyGenericError, WriteToOperation, err) } return count, nil } @@ -359,7 +359,7 @@ func (c *quicErrWrapperUDPLikeConn) WriteTo(p []byte, addr net.Addr) (int, error func (c *quicErrWrapperUDPLikeConn) ReadFrom(b []byte) (int, net.Addr, error) { n, addr, err := c.UDPLikeConn.ReadFrom(b) if err != nil { - return 0, nil, NewErrWrapper(ClassifyGenericError, ReadFromOperation, err) + return 0, nil, NewErrWrapper(classifyGenericError, ReadFromOperation, err) } return n, addr, nil } @@ -368,7 +368,7 @@ func (c *quicErrWrapperUDPLikeConn) ReadFrom(b []byte) (int, net.Addr, error) { func (c *quicErrWrapperUDPLikeConn) Close() error { err := c.UDPLikeConn.Close() if err != nil { - return NewErrWrapper(ClassifyGenericError, ReadFromOperation, err) + return NewErrWrapper(classifyGenericError, ReadFromOperation, err) } return nil } @@ -385,7 +385,7 @@ func (d *quicDialerErrWrapper) DialContext( sess, err := d.QUICDialer.DialContext(ctx, network, host, tlsCfg, cfg) if err != nil { return nil, NewErrWrapper( - ClassifyQUICHandshakeError, QUICHandshakeOperation, err) + classifyQUICHandshakeError, QUICHandshakeOperation, err) } return sess, nil } diff --git a/internal/netxlite/quirks_test.go b/internal/netxlite/quirks_test.go index 182ae1a..93db8b4 100644 --- a/internal/netxlite/quirks_test.go +++ b/internal/netxlite/quirks_test.go @@ -35,12 +35,12 @@ func TestQuirkReduceErrors(t *testing.T) { t.Run("multiple errors with meaningful ones", func(t *testing.T) { err1 := errors.New("mocked error #1") err2 := NewErrWrapper( - ClassifyGenericError, + classifyGenericError, CloseOperation, errors.New("antani"), ) err3 := NewErrWrapper( - ClassifyGenericError, + classifyGenericError, CloseOperation, ECONNREFUSED, ) diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go index 5091f94..ba1fc15 100644 --- a/internal/netxlite/resolver.go +++ b/internal/netxlite/resolver.go @@ -248,7 +248,7 @@ var _ model.Resolver = &resolverErrWrapper{} func (r *resolverErrWrapper) LookupHost(ctx context.Context, hostname string) ([]string, error) { addrs, err := r.Resolver.LookupHost(ctx, hostname) if err != nil { - return nil, NewErrWrapper(ClassifyResolverError, ResolveOperation, err) + return nil, NewErrWrapper(classifyResolverError, ResolveOperation, err) } return addrs, nil } @@ -257,7 +257,7 @@ func (r *resolverErrWrapper) LookupHTTPS( ctx context.Context, domain string) (*model.HTTPSSvc, error) { out, err := r.Resolver.LookupHTTPS(ctx, domain) if err != nil { - return nil, NewErrWrapper(ClassifyResolverError, ResolveOperation, err) + return nil, NewErrWrapper(classifyResolverError, ResolveOperation, err) } return out, nil } diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index ad51228..f0a2fa5 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -335,7 +335,7 @@ func (h *tlsHandshakerErrWrapper) Handshake( tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) if err != nil { return nil, tls.ConnectionState{}, NewErrWrapper( - ClassifyTLSHandshakeError, TLSHandshakeOperation, err) + classifyTLSHandshakeError, TLSHandshakeOperation, err) } return tlsconn, state, nil } diff --git a/internal/ptx/obfs4_test.go b/internal/ptx/obfs4_test.go index 3f22653..f3a5876 100644 --- a/internal/ptx/obfs4_test.go +++ b/internal/ptx/obfs4_test.go @@ -13,8 +13,10 @@ import ( ) func TestOBFS4DialerWorks(t *testing.T) { - // This test is 0.3 seconds in my machine, so it's ~fine - // to run it even when we're in short mode + if testing.Short() { + // Was failing in https://github.com/ooni/probe-cli/pull/655 + t.Skip("skip test in short mode") + } o4d := DefaultTestingOBFS4Bridge() conn, err := o4d.DialContext(context.Background()) if err != nil { @@ -103,6 +105,10 @@ func (c *obfs4connwrapper) Close() error { } func TestOBFS4DialerWorksWithContextExpiration(t *testing.T) { + if testing.Short() { + // Was failing in https://github.com/ooni/probe-cli/pull/655 + t.Skip("skip test in short mode") + } ctx, cancel := context.WithCancel(context.Background()) defer cancel() called := &atomicx.Int64{}