diff --git a/internal/tracex/archival.go b/internal/tracex/archival.go index 9f9a3e6..6f95bb2 100644 --- a/internal/tracex/archival.go +++ b/internal/tracex/archival.go @@ -64,8 +64,8 @@ func NewTCPConnectList(begin time.Time, events []Event) (out []TCPConnectEntry) Port: iport, Status: TCPConnectStatus{ Blocked: nil, // only used by Web Connectivity - Failure: NewFailure(event.Err), - Success: event.Err == nil, + Failure: event.Err.ToFailure(), + Success: event.Err.IsNil(), }, T: event.Time.Sub(begin).Seconds(), }) @@ -73,21 +73,10 @@ func NewTCPConnectList(begin time.Time, events []Event) (out []TCPConnectEntry) return } -// NewFailure creates a failure nullable string from the given error +// NewFailure creates a failure nullable string from the given error. This function +// is equivalent to NewFailureStr(err).ToFailure(). func NewFailure(err error) *string { - if err == nil { - return nil - } - // 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 = netxlite.NewTopLevelGenericErrWrapper(err) - errWrapper := err.(*netxlite.ErrWrapper) - s := errWrapper.Failure - if s == "" { - s = "unknown_failure: errWrapper.Failure is empty" - } - return &s + return NewFailureStr(err).ToFailure() } // NewFailedOperation creates a failed operation string from the given error. @@ -158,7 +147,7 @@ func newRequestList(begin time.Time, events []Event) (out []RequestEntry) { entry.Response.Locations = ev.HTTPResponseHeaders.Values("Location") entry.Response.Body.Value = string(ev.HTTPResponseBody) entry.Response.BodyIsTruncated = ev.HTTPResponseBodyIsTruncated - entry.Failure = NewFailure(ev.Err) + entry.Failure = ev.Err.ToFailure() out = append(out, entry) } } @@ -183,7 +172,7 @@ func NewDNSQueriesList(begin time.Time, events []Event) (out []DNSQueryEntry) { entry.Answers, qtype.makeAnswerEntry(addr)) } } - if len(entry.Answers) <= 0 && ev.Err == nil { + if len(entry.Answers) <= 0 && ev.Err.IsNil() { // This allows us to skip cases where the server does not have // an IPv6 address but has an IPv4 address. Instead, when we // receive an error, we want to track its existence. The main @@ -228,7 +217,7 @@ func (qtype dnsQueryType) makeAnswerEntry(addr string) DNSAnswerEntry { func (qtype dnsQueryType) makeQueryEntry(begin time.Time, ev *EventValue) DNSQueryEntry { return DNSQueryEntry{ Engine: ev.Proto, - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Hostname: ev.Hostname, QueryType: string(qtype), ResolverAddress: ev.Address, @@ -244,21 +233,21 @@ func NewNetworkEventsList(begin time.Time, events []Event) (out []NetworkEvent) case *EventConnectOperation: out = append(out, NetworkEvent{ Address: ev.Address, - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), Proto: ev.Proto, T: ev.Time.Sub(begin).Seconds(), }) case *EventReadOperation: out = append(out, NetworkEvent{ - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), NumBytes: int64(ev.NumBytes), T: ev.Time.Sub(begin).Seconds(), }) case *EventWriteOperation: out = append(out, NetworkEvent{ - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), NumBytes: int64(ev.NumBytes), T: ev.Time.Sub(begin).Seconds(), @@ -266,7 +255,7 @@ func NewNetworkEventsList(begin time.Time, events []Event) (out []NetworkEvent) case *EventReadFromOperation: out = append(out, NetworkEvent{ Address: ev.Address, - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), NumBytes: int64(ev.NumBytes), T: ev.Time.Sub(begin).Seconds(), @@ -274,14 +263,14 @@ func NewNetworkEventsList(begin time.Time, events []Event) (out []NetworkEvent) case *EventWriteToOperation: out = append(out, NetworkEvent{ Address: ev.Address, - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), NumBytes: int64(ev.NumBytes), T: ev.Time.Sub(begin).Seconds(), }) default: // For example, "tls_handshake_done" (used in data analysis!) out = append(out, NetworkEvent{ - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), Operation: wrapper.Name(), T: ev.Time.Sub(begin).Seconds(), }) @@ -302,7 +291,7 @@ func NewTLSHandshakesList(begin time.Time, events []Event) (out []TLSHandshake) out = append(out, TLSHandshake{ Address: ev.Address, CipherSuite: ev.TLSCipherSuite, - Failure: NewFailure(ev.Err), + Failure: ev.Err.ToFailure(), NegotiatedProtocol: ev.TLSNegotiatedProto, NoTLSVerify: ev.NoTLSVerify, PeerCertificates: tlsMakePeerCerts(ev.TLSPeerCerts), diff --git a/internal/tracex/archival_test.go b/internal/tracex/archival_test.go index ade7391..479b32f 100644 --- a/internal/tracex/archival_test.go +++ b/internal/tracex/archival_test.go @@ -92,7 +92,7 @@ func TestNewTCPConnectList(t *testing.T) { }}, &EventConnectOperation{&EventValue{ Address: "8.8.4.4:53", Duration: 50 * time.Millisecond, - Err: io.EOF, + Err: netxlite.FailureEOFError, Proto: "tcp", Time: begin.Add(180 * time.Millisecond), }}}, @@ -165,7 +165,7 @@ func TestNewRequestList(t *testing.T) { }, HTTPMethod: "GET", HTTPURL: "https://www.example.com/result", - Err: io.EOF, + Err: netxlite.FailureEOFError, Time: begin.Add(20 * time.Millisecond), }}}, }, @@ -332,7 +332,7 @@ func TestNewDNSQueriesList(t *testing.T) { }}, &EventConnectOperation{&EventValue{ // skipped because not relevant Address: "8.8.4.4:53", Duration: 50 * time.Millisecond, - Err: io.EOF, + Err: netxlite.FailureEOFError, Proto: "tcp", Time: begin.Add(180 * time.Millisecond), }}}, @@ -381,7 +381,7 @@ func TestNewDNSQueriesList(t *testing.T) { args: args{ begin: begin, events: []Event{&EventResolveDone{&EventValue{ - Err: &netxlite.ErrWrapper{Failure: netxlite.FailureDNSNXDOMAINError}, + Err: netxlite.FailureDNSNXDOMAINError, Hostname: "dns.google.com", Time: begin.Add(200 * time.Millisecond), }}}, @@ -435,25 +435,25 @@ func TestNewNetworkEventsList(t *testing.T) { begin: begin, events: []Event{&EventConnectOperation{&EventValue{ Address: "8.8.8.8:853", - Err: io.EOF, + Err: netxlite.FailureEOFError, Proto: "tcp", Time: begin.Add(7 * time.Millisecond), }}, &EventReadOperation{&EventValue{ - Err: context.Canceled, + Err: netxlite.FailureInterrupted, NumBytes: 7117, Time: begin.Add(11 * time.Millisecond), }}, &EventReadFromOperation{&EventValue{ Address: "8.8.8.8:853", - Err: context.Canceled, + Err: netxlite.FailureInterrupted, NumBytes: 7117, Time: begin.Add(11 * time.Millisecond), }}, &EventWriteOperation{&EventValue{ - Err: websocket.ErrBadHandshake, + Err: NewFailureStr(websocket.ErrBadHandshake), NumBytes: 4114, Time: begin.Add(14 * time.Millisecond), }}, &EventWriteToOperation{&EventValue{ Address: "8.8.8.8:853", - Err: websocket.ErrBadHandshake, + Err: NewFailureStr(websocket.ErrBadHandshake), NumBytes: 4114, Time: begin.Add(14 * time.Millisecond), }}, &EventResolveStart{&EventValue{ @@ -528,7 +528,7 @@ func TestNewTLSHandshakesList(t *testing.T) { begin: begin, events: []Event{&EventTLSHandshakeDone{&EventValue{ Address: "131.252.210.176:443", - Err: io.EOF, + Err: netxlite.FailureEOFError, NoTLSVerify: false, Proto: "tcp", TLSCipherSuite: "SUITE", @@ -564,7 +564,7 @@ func TestNewTLSHandshakesList(t *testing.T) { begin: begin, events: []Event{&EventQUICHandshakeDone{&EventValue{ Address: "131.252.210.176:443", - Err: io.EOF, + Err: netxlite.FailureEOFError, NoTLSVerify: false, Proto: "quic", TLSCipherSuite: "SUITE", diff --git a/internal/tracex/dialer.go b/internal/tracex/dialer.go index 8820850..48e9a0e 100644 --- a/internal/tracex/dialer.go +++ b/internal/tracex/dialer.go @@ -54,7 +54,7 @@ func (d *DialerSaver) DialContext(ctx context.Context, network, address string) d.Saver.Write(&EventConnectOperation{&EventValue{ Address: address, Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), Proto: network, Time: stop, }}) @@ -128,7 +128,7 @@ func (c *dialerConnWrapper) Read(p []byte) (int, error) { Address: remoteAddr, Data: p[:count], Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NumBytes: count, Proto: proto, Time: stop, @@ -146,7 +146,7 @@ func (c *dialerConnWrapper) Write(p []byte) (int, error) { Address: remoteAddr, Data: p[:count], Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NumBytes: count, Proto: proto, Time: stop, diff --git a/internal/tracex/dialer_test.go b/internal/tracex/dialer_test.go index 2447ce4..2bcd471 100644 --- a/internal/tracex/dialer_test.go +++ b/internal/tracex/dialer_test.go @@ -57,7 +57,7 @@ func TestDialerSaver(t *testing.T) { if ev[0].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if !errors.Is(ev[0].Value().Err, expected) { + if ev[0].Value().Err != "unknown_failure: mocked error" { t.Fatal("unexpected Err") } if ev[0].Name() != netxlite.ConnectOperation { @@ -210,7 +210,7 @@ func TestDialerConnWrapper(t *testing.T) { if ev[0].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if !errors.Is(ev[0].Value().Err, io.EOF) { + if ev[0].Value().Err != netxlite.FailureEOFError { t.Fatal("unexpected Err") } if ev[0].Name() != netxlite.ReadOperation { @@ -263,7 +263,7 @@ func TestDialerConnWrapper(t *testing.T) { if ev[0].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if !errors.Is(ev[0].Value().Err, io.EOF) { + if ev[0].Value().Err != netxlite.FailureEOFError { t.Fatal("unexpected Err") } if ev[0].Name() != netxlite.WriteOperation { diff --git a/internal/tracex/event.go b/internal/tracex/event.go index 48f2b71..13c43b4 100644 --- a/internal/tracex/event.go +++ b/internal/tracex/event.go @@ -6,12 +6,69 @@ package tracex import ( "crypto/x509" + "errors" "net/http" "time" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) +// FailureStr is the string representation of an error. The empty +// string represents the absence of any error. +type FailureStr string + +// NewFailureStr creates a FailureStr from an error. If the error is not +// already an ErrWrapper, it's converted to an ErrWrapper. If the ErrWrapper's +// Failure is not empty, we return that. Otherwise we return a string +// indicating that an ErrWrapper has an empty failure (should not happen). +func NewFailureStr(err error) FailureStr { + if err == nil { + return "" + } + // 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 could happen is with context deadline for HTTP when you + // have wrapped the underlying dialers but not the Transport. + var errWrapper *netxlite.ErrWrapper + if !errors.As(err, &errWrapper) { + err := netxlite.NewTopLevelGenericErrWrapper(err) + couldConvert := errors.As(err, &errWrapper) + runtimex.PanicIfFalse(couldConvert, "we should have an ErrWrapper here") + } + s := errWrapper.Failure + if s == "" { + s = "unknown_failure: errWrapper.Failure is empty" + } + return FailureStr(s) +} + +// IsNil returns whether this FailureStr is nil. Technically speaking, the +// failure cannot be nil, but an empty string is equivalent to nil after +// we convert using ToFailure(). Also, this type is often called Err, Error, +// or Failure. So, the resulting code actually reads correct. +func (fs FailureStr) IsNil() bool { + return fs == "" +} + +// IsNotNil is the opposite of IsNil. Technically speaking, the +// failure cannot be nil, but an empty string is equivalent to nil after +// we convert using ToFailure(). Also, this type is often called Err, Error, +// or Failure. So, the resulting code actually reads correct. +func (fs FailureStr) IsNotNil() bool { + return !fs.IsNil() +} + +// ToFailure converts a FailureStr to a OONI failure (i.e., a string +// on error and nil in case of success). +func (fs FailureStr) ToFailure() (out *string) { + if fs != "" { + s := string(fs) + out = &s + } + return +} + // Event is one of the events within a trace. type Event interface { // Value returns the event value @@ -224,7 +281,7 @@ type EventValue struct { DNSResponse []byte `json:",omitempty"` Data []byte `json:",omitempty"` Duration time.Duration `json:",omitempty"` - Err error `json:",omitempty"` + Err FailureStr `json:",omitempty"` HTTPMethod string `json:",omitempty"` HTTPRequestHeaders http.Header `json:",omitempty"` HTTPResponseHeaders http.Header `json:",omitempty"` diff --git a/internal/tracex/http.go b/internal/tracex/http.go index 9d745aa..58d1205 100644 --- a/internal/tracex/http.go +++ b/internal/tracex/http.go @@ -73,7 +73,7 @@ func (txp *HTTPTransportSaver) RoundTrip(req *http.Request) (*http.Response, err if err != nil { ev.Duration = time.Since(started) - ev.Err = err + ev.Err = NewFailureStr(err) return nil, err } @@ -86,7 +86,7 @@ func (txp *HTTPTransportSaver) RoundTrip(req *http.Request) (*http.Response, err if err != nil { ev.Duration = time.Since(started) - ev.Err = err + ev.Err = NewFailureStr(err) return nil, err } diff --git a/internal/tracex/http_test.go b/internal/tracex/http_test.go index 0efcae3..67db7c7 100644 --- a/internal/tracex/http_test.go +++ b/internal/tracex/http_test.go @@ -178,7 +178,7 @@ func TestHTTPTransportSaver(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected nonzero duration") } - if value.Err.Error() != "connection_reset" { + if value.Err != netxlite.FailureConnectionReset { t.Fatal("unexpected Err value") } if len(value.HTTPResponseHeaders) > 0 { @@ -268,7 +268,7 @@ func TestHTTPTransportSaver(t *testing.T) { if ev[1].Value().HTTPResponseHeaders.Get("Server") != "antani" { t.Fatal("invalid Server header") } - if ev[1].Value().Err.Error() != "unknown_failure: mocked error" { + if ev[1].Value().Err != "unknown_failure: mocked error" { t.Fatal("invalid error") } }) diff --git a/internal/tracex/quic.go b/internal/tracex/quic.go index 2162ec6..0d9882e 100644 --- a/internal/tracex/quic.go +++ b/internal/tracex/quic.go @@ -60,7 +60,7 @@ func (h *QUICDialerSaver) DialContext(ctx context.Context, network string, h.Saver.Write(&EventQUICHandshakeDone{&EventValue{ Address: host, Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NoTLSVerify: tlsCfg.InsecureSkipVerify, Proto: network, TLSNextProtos: tlsCfg.NextProtos, @@ -149,7 +149,7 @@ func (c *quicPacketConnWrapper) WriteTo(p []byte, addr net.Addr) (int, error) { Address: addr.String(), Data: p[:count], Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NumBytes: count, Time: stop, }}) @@ -168,7 +168,7 @@ func (c *quicPacketConnWrapper) ReadFrom(b []byte) (int, net.Addr, error) { Address: c.safeAddrString(addr), Data: data, Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NumBytes: n, Time: stop, }}) diff --git a/internal/tracex/quic_test.go b/internal/tracex/quic_test.go index 4f766ed..8e54b0b 100644 --- a/internal/tracex/quic_test.go +++ b/internal/tracex/quic_test.go @@ -50,7 +50,7 @@ func TestQUICDialerSaver(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected non-zero duration") } - if value.Err != nil { + if value.Err.IsNotNil() { t.Fatal("expected no error here") } if value.TLSCipherSuite != "TLS_RSA_WITH_RC4_128_SHA" { @@ -120,7 +120,7 @@ func TestQUICDialerSaver(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected non-zero duration") } - if value.Err == nil { + if value.Err.IsNil() { t.Fatal("expected non-nil error here") } } @@ -266,7 +266,7 @@ func TestQUICPacketConnWrapper(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected nonzero duration") } - if !errors.Is(value.Err, expected) { + if value.Err != "unknown_failure: mocked error" { t.Fatal("unexpected value.Err", value.Err) } if value.NumBytes != 0 { @@ -326,7 +326,7 @@ func TestQUICPacketConnWrapper(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected nonzero duration") } - if value.Err != nil { + if value.Err.IsNotNil() { t.Fatal("unexpected value.Err", value.Err) } if value.NumBytes != 4 { @@ -382,7 +382,7 @@ func TestQUICPacketConnWrapper(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected nonzero duration") } - if !errors.Is(value.Err, expected) { + if value.Err != "unknown_failure: mocked error" { t.Fatal("unexpected value.Err", value.Err) } if value.NumBytes != 0 { @@ -434,7 +434,7 @@ func TestQUICPacketConnWrapper(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected nonzero duration") } - if value.Err != nil { + if value.Err.IsNotNil() { t.Fatal("unexpected value.Err", value.Err) } if value.NumBytes != 1 { diff --git a/internal/tracex/resolver.go b/internal/tracex/resolver.go index 62cf1f5..9cc6af8 100644 --- a/internal/tracex/resolver.go +++ b/internal/tracex/resolver.go @@ -51,7 +51,7 @@ func (r *ResolverSaver) LookupHost(ctx context.Context, hostname string) ([]stri Addresses: addrs, Address: r.Resolver.Address(), Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), Hostname: hostname, Proto: r.Resolver.Network(), Time: stop, @@ -122,7 +122,7 @@ func (txp *DNSTransportSaver) RoundTrip( DNSQuery: dnsMaybeQueryBytes(query), DNSResponse: dnsMaybeResponseBytes(response), Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), Proto: txp.DNSTransport.Network(), Time: stop, }}) diff --git a/internal/tracex/resolver_test.go b/internal/tracex/resolver_test.go index 631ee64..d71785b 100644 --- a/internal/tracex/resolver_test.go +++ b/internal/tracex/resolver_test.go @@ -11,12 +11,13 @@ import ( "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" "github.com/ooni/probe-cli/v3/internal/runtimex" ) func TestResolverSaver(t *testing.T) { t.Run("on failure", func(t *testing.T) { - expected := errors.New("no such host") + expected := netxlite.ErrOODNSNoSuchHost saver := &Saver{} reso := saver.WrapResolver(newFakeResolverWithExplicitError(expected)) addrs, err := reso.LookupHost(context.Background(), "www.google.com") @@ -45,7 +46,7 @@ func TestResolverSaver(t *testing.T) { if ev[1].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if !errors.Is(ev[1].Value().Err, expected) { + if ev[1].Value().Err != netxlite.FailureDNSNXDOMAINError { t.Fatal("unexpected Err") } if ev[1].Value().Hostname != "www.google.com" { @@ -89,7 +90,7 @@ func TestResolverSaver(t *testing.T) { if ev[1].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if ev[1].Value().Err != nil { + if ev[1].Value().Err.IsNotNil() { t.Fatal("unexpected Err") } if ev[1].Value().Hostname != "www.google.com" { @@ -106,7 +107,7 @@ func TestResolverSaver(t *testing.T) { func TestDNSTransportSaver(t *testing.T) { t.Run("on failure", func(t *testing.T) { - expected := errors.New("no such host") + expected := netxlite.ErrOODNSNoSuchHost saver := &Saver{} txp := saver.WrapDNSTransport(&mocks.DNSTransport{ MockRoundTrip: func(ctx context.Context, query model.DNSQuery) (model.DNSResponse, error) { @@ -154,7 +155,7 @@ func TestDNSTransportSaver(t *testing.T) { if ev[1].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if !errors.Is(ev[1].Value().Err, expected) { + if ev[1].Value().Err != netxlite.FailureDNSNXDOMAINError { t.Fatal("unexpected Err") } if ev[1].Name() != "dns_round_trip_done" { @@ -219,7 +220,7 @@ func TestDNSTransportSaver(t *testing.T) { if ev[1].Value().Duration <= 0 { t.Fatal("unexpected Duration") } - if ev[1].Value().Err != nil { + if ev[1].Value().Err.IsNotNil() { t.Fatal("unexpected Err") } if ev[1].Name() != "dns_round_trip_done" { diff --git a/internal/tracex/tls.go b/internal/tracex/tls.go index 02ea359..128b27d 100644 --- a/internal/tracex/tls.go +++ b/internal/tracex/tls.go @@ -59,7 +59,7 @@ func (h *TLSHandshakerSaver) Handshake( h.Saver.Write(&EventTLSHandshakeDone{&EventValue{ Address: remoteAddr, Duration: stop.Sub(start), - Err: err, + Err: NewFailureStr(err), NoTLSVerify: config.InsecureSkipVerify, Proto: proto, TLSCipherSuite: netxlite.TLSCipherSuiteString(state.CipherSuite), diff --git a/internal/tracex/tls_test.go b/internal/tracex/tls_test.go index fc0eaf5..86f0319 100644 --- a/internal/tracex/tls_test.go +++ b/internal/tracex/tls_test.go @@ -48,7 +48,7 @@ func TestTLSHandshakerSaver(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected non-zero duration") } - if value.Err != nil { + if value.Err.IsNotNil() { t.Fatal("expected no error here") } if value.TLSCipherSuite != "TLS_RSA_WITH_RC4_128_SHA" { @@ -130,7 +130,7 @@ func TestTLSHandshakerSaver(t *testing.T) { if value.Duration <= 0 { t.Fatal("expected non-zero duration") } - if value.Err == nil { + if value.Err.IsNil() { t.Fatal("expected non-nil error here") } if value.TLSCipherSuite != "" {