refactor(netxlite/errors): improve docs and format code (#481)

No real functional change. A few are needed and they will come
next. With this diff I just wanted to do cosmetic changes and
documentation changes, to ensure this package is okay.

See https://github.com/ooni/probe/issues/1591

internal/netxlite/errorsx/classify.go

@@ -18,6 +18,26 @@ import (
 
 // ClassifyGenericError is the generic classifier mapping an error
 // occurred during an operation to an OONI failure string.
+//
+// If the input error is already an ErrWrapper we don't perform
+// the classification again and we return its Failure to the caller.
+//
+// Classification rules
+//
+// We put inside this classifier:
+//
+// - system call errors
+//
+// - generic errors that can occur in multiple places
+//
+// - all the errors that depend on strings
+//
+// The more specific classifiers will call this classifier if
+// they fail to find a mapping for the input error.
+//
+// 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 its value.
 func ClassifyGenericError(err error) string {
 	// The list returned here matches the values used by MK unless
 	// explicitly noted otherwise with a comment.
@@ -27,6 +47,9 @@ func ClassifyGenericError(err error) string {
 		return errwrapper.Error() // we've already wrapped it
 	}
 
+	// Classify system errors first. We could use strings for many
+	// of them on Unix, but this would fail on Windows as described
+	// by https://github.com/ooni/probe/issues/1526.
 	if failure := classifySyscallError(err); failure != "" {
 		return failure
 	}
@@ -34,6 +57,7 @@ func ClassifyGenericError(err error) string {
 	if errors.Is(err, context.Canceled) {
 		return FailureInterrupted
 	}
+
 	s := err.Error()
 	if strings.HasSuffix(s, "operation was canceled") {
 		return FailureInterrupted
@@ -50,7 +74,6 @@ func ClassifyGenericError(err error) string {
 	if strings.HasSuffix(s, "i/o timeout") {
 		return FailureGenericTimeoutError
 	}
-	// TODO(kelmenhorst,bassosimone): this can probably be moved since it's TLS specific
 	if strings.HasSuffix(s, "TLS handshake timeout") {
 		return FailureGenericTimeoutError
 	}
@@ -60,11 +83,13 @@ func ClassifyGenericError(err error) string {
 		// that we return here is significantly more specific.
 		return FailureDNSNXDOMAINError
 	}
+
 	formatted := fmt.Sprintf("unknown_failure: %s", s)
 	return scrubber.Scrub(formatted) // scrub IP addresses in the error
 }
 
-// TLS alert protocol as defined in RFC8446
+// TLS alert protocol as defined in RFC8446. We need these definitions
+// to figure out which error occurred during a QUIC handshake.
 const (
 	// Sender was unable to negotiate an acceptable set of security parameters given the options available.
 	quicTLSAlertHandshakeFailure = 40
@@ -94,6 +119,11 @@ const (
 	quicTLSUnrecognizedName = 112
 )
 
+// quicIsCertificateError tells us whether a specific TLS alert error
+// we received is actually an error depending on the certificate.
+//
+// The set of checks we implement here is a set of heuristics based
+// on our understanding of the TLS spec and may need tweaks.
 func quicIsCertificateError(alert uint8) bool {
 	return (alert == quicTLSAlertBadCertificate ||
 		alert == quicTLSAlertUnsupportedCertificate ||
@@ -104,17 +134,25 @@ func quicIsCertificateError(alert uint8) bool {
 
 // ClassifyQUICHandshakeError maps an error occurred during the QUIC
 // handshake to an OONI failure string.
+//
+// If the input error is already an ErrWrapper we don't perform
+// the classification again and we return its Failure to the caller.
+//
+// If this classifier fails, it calls ClassifyGenericError and
+// returns to the caller its return value.
 func ClassifyQUICHandshakeError(err error) string {
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
 		return errwrapper.Error() // we've already wrapped it
 	}
 
-	var versionNegotiation *quic.VersionNegotiationError
+	var (
-	var statelessReset *quic.StatelessResetError
+		versionNegotiation *quic.VersionNegotiationError
-	var handshakeTimeout *quic.HandshakeTimeoutError
+		statelessReset     *quic.StatelessResetError
-	var idleTimeout *quic.IdleTimeoutError
+		handshakeTimeout   *quic.HandshakeTimeoutError
-	var transportError *quic.TransportError
+		idleTimeout        *quic.IdleTimeoutError
+		transportError     *quic.TransportError
+	)
 
 	if errors.As(err, &versionNegotiation) {
 		return FailureQUICIncompatibleVersion
@@ -137,8 +175,9 @@ func ClassifyQUICHandshakeError(err error) string {
 		if quicIsCertificateError(errCode) {
 			return FailureSSLInvalidCertificate
 		}
-		// TLSAlertDecryptError and TLSAlertHandshakeFailure are summarized to a FailureSSLHandshake error because both
+		// TLSAlertDecryptError and TLSAlertHandshakeFailure are summarized to a
-		// alerts are caused by a failed or corrupted parameter negotiation during the TLS handshake.
+		// FailureSSLHandshake error because both alerts are caused by a failed or
+		// corrupted parameter negotiation during the TLS handshake.
 		if errCode == quicTLSAlertDecryptError || errCode == quicTLSAlertHandshakeFailure {
 			return FailureSSLFailedHandshake
 		}
@@ -152,13 +191,18 @@ func ClassifyQUICHandshakeError(err error) string {
 	return ClassifyGenericError(err)
 }
 
-// ErrDNSBogon indicates that we found a bogon address. This is the
+// ErrDNSBogon indicates that we found a bogon address. Code that
-// correct value with which to initialize MeasurementRoot.ErrDNSBogon
+// filters for DNS bogons MUST use this error.
-// to tell this library to return an error when a bogon is found.
 var ErrDNSBogon = errors.New("dns: detected bogon address")
 
 // ClassifyResolverError maps an error occurred during a domain name
 // resolution to the corresponding OONI failure string.
+//
+// If the input error is already an ErrWrapper we don't perform
+// the classification again and we return its Failure to the caller.
+//
+// If this classifier fails, it calls ClassifyGenericError and
+// returns to the caller its return value.
 func ClassifyResolverError(err error) string {
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
@@ -172,6 +216,12 @@ func ClassifyResolverError(err error) string {
 
 // ClassifyTLSHandshakeError maps an error occurred during the TLS
 // handshake to an OONI failure string.
+//
+// If the input error is already an ErrWrapper we don't perform
+// the classification again and we return its Failure to the caller.
+//
+// If this classifier fails, it calls ClassifyGenericError and
+// returns to the caller its return value.
 func ClassifyTLSHandshakeError(err error) string {
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {

internal/netxlite/errorsx/classify_test.go

@@ -22,52 +22,62 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal("did not classify existing ErrWrapper correctly")
 		}
 	})
+
 	t.Run("for already wrapped error", func(t *testing.T) {
 		err := io.EOF
 		if ClassifyGenericError(err) != FailureEOFError {
 			t.Fatal("unexpected result")
 		}
 	})
+
 	t.Run("for context.Canceled", func(t *testing.T) {
 		if ClassifyGenericError(context.Canceled) != FailureInterrupted {
 			t.Fatal("unexpected result")
 		}
 	})
+
 	t.Run("for operation was canceled error", func(t *testing.T) {
 		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 {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for canceled", func(t *testing.T) {
 		if ClassifyGenericError(syscall.ECANCELED) != FailureOperationCanceled {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for connection_refused", func(t *testing.T) {
 		if ClassifyGenericError(syscall.ECONNREFUSED) != FailureConnectionRefused {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for connection_reset", func(t *testing.T) {
 		if ClassifyGenericError(syscall.ECONNRESET) != FailureConnectionReset {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for host_unreachable", func(t *testing.T) {
 		if ClassifyGenericError(syscall.EHOSTUNREACH) != FailureHostUnreachable {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for system timeout", func(t *testing.T) {
 		if ClassifyGenericError(syscall.ETIMEDOUT) != FailureTimedOut {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for context deadline exceeded", func(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), 1)
 		defer cancel()
@@ -76,11 +86,13 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for stun's transaction is timed out", func(t *testing.T) {
 		if ClassifyGenericError(stun.ErrTransactionTimeOut) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for i/o error", func(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), 1)
 		defer cancel() // fail immediately
@@ -95,12 +107,14 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for TLS handshake timeout error", func(t *testing.T) {
 		err := errors.New("net/http: TLS handshake timeout")
 		if ClassifyGenericError(err) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for no such host", func(t *testing.T) {
 		if ClassifyGenericError(&net.DNSError{
 			Err: "no such host",
@@ -108,6 +122,7 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for errors including IPv4 address", func(t *testing.T) {
 		input := errors.New("read tcp 10.0.2.15:56948->93.184.216.34:443: use of closed network connection")
 		expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection"
@@ -116,6 +131,7 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal(cmp.Diff(expected, out))
 		}
 	})
+
 	t.Run("for errors including IPv6 address", func(t *testing.T) {
 		input := errors.New("read tcp [::1]:56948->[::1]:443: use of closed network connection")
 		expected := "unknown_failure: read tcp [scrubbed]->[scrubbed]: use of closed network connection"
@@ -124,6 +140,7 @@ func TestClassifyGenericError(t *testing.T) {
 			t.Fatal(cmp.Diff(expected, out))
 		}
 	})
+
 	t.Run("for i/o error", func(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), 1)
 		defer cancel() // fail immediately
@@ -152,55 +169,65 @@ func TestClassifyQUICHandshakeError(t *testing.T) {
 			t.Fatal("did not classify existing ErrWrapper correctly")
 		}
 	})
+
 	t.Run("for connection_reset", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.StatelessResetError{}) != FailureConnectionReset {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for incompatible quic version", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.VersionNegotiationError{}) != FailureQUICIncompatibleVersion {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for quic connection refused", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: quic.ConnectionRefused}) != FailureConnectionRefused {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for quic handshake timeout", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.HandshakeTimeoutError{}) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for QUIC idle connection timeout", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(&quic.IdleTimeoutError{}) != FailureGenericTimeoutError {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for QUIC CRYPTO Handshake", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSAlertHandshakeFailure
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLFailedHandshake {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for QUIC CRYPTO Invalid Certificate", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSAlertBadCertificate
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidCertificate {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for QUIC CRYPTO Unknown CA", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSAlertUnknownCA
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLUnknownAuthority {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for QUIC CRYPTO Bad Hostname", func(t *testing.T) {
 		var err quic.TransportErrorCode = quicTLSUnrecognizedName
 		if ClassifyQUICHandshakeError(&quic.TransportError{ErrorCode: err}) != FailureSSLInvalidHostname {
 			t.Fatal("unexpected results")
 		}
 	})
+
 	t.Run("for another kind of error", func(t *testing.T) {
 		if ClassifyQUICHandshakeError(io.EOF) != FailureEOFError {
 			t.Fatal("unexpected result")
@@ -215,11 +242,13 @@ func TestClassifyResolverError(t *testing.T) {
 			t.Fatal("did not classify existing ErrWrapper correctly")
 		}
 	})
+
 	t.Run("for ErrDNSBogon", func(t *testing.T) {
 		if ClassifyResolverError(ErrDNSBogon) != FailureDNSBogonError {
 			t.Fatal("unexpected result")
 		}
 	})
+
 	t.Run("for another kind of error", func(t *testing.T) {
 		if ClassifyResolverError(io.EOF) != FailureEOFError {
 			t.Fatal("unexpected result")
@@ -234,24 +263,28 @@ func TestClassifyTLSHandshakeError(t *testing.T) {
 			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 {
 			t.Fatal("unexpected result")
 		}
 	})
+
 	t.Run("for x509.UnknownAuthorityError", func(t *testing.T) {
 		var err x509.UnknownAuthorityError
 		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 {
 			t.Fatal("unexpected result")
 		}
 	})
+
 	t.Run("for another kind of error", func(t *testing.T) {
 		if ClassifyTLSHandshakeError(io.EOF) != FailureEOFError {
 			t.Fatal("unexpected result")

internal/netxlite/errorsx/doc.go

@@ -1,2 +1,20 @@
 // Package errorsx contains code to classify errors.
+//
+// We define the ErrWrapper type, that should wrap any error
+// and map it to the corresponding OONI failure.
+//
+// See https://github.com/ooni/spec/blob/master/data-formats/df-007-errors.md
+// for a list of OONI failure strings.
+//
+// We define ClassifyXXX functions that map an `error` type to
+// the corresponding OONI failure.
+//
+// When we cannot map an error to an OONI failure we return
+// an "unknown_failure: XXX" string where the XXX part has
+// been scrubbed so to remove any network endpoints.
+//
+// The general approach we have been following for this
+// package has been to return the same strings that we used
+// with the previous measurement engine, Measurement Kit
+// available at https://github.com/measurement-kit/measurement-kit.
 package errorsx

internal/netxlite/errorsx/errno.go

@@ -1,5 +1,5 @@
 // Code generated by go generate; DO NOT EDIT.
-// Generated: 2021-09-07 16:43:08.462721 +0200 CEST m=+0.105415376
+// Generated: 2021-09-07 20:26:06.502417 +0200 CEST m=+0.133209876
 
 package errorsx
 

internal/netxlite/errorsx/errno_test.go

@@ -1,5 +1,5 @@
 // Code generated by go generate; DO NOT EDIT.
-// Generated: 2021-09-07 16:43:08.510432 +0200 CEST m=+0.153127376
+// Generated: 2021-09-07 20:26:06.547786 +0200 CEST m=+0.178579584
 
 package errorsx
 
@@ -9,95 +9,184 @@ import (
 	"testing"
 )
 
-func TestToSyscallErr(t *testing.T) {
+func TestClassifySyscallError(t *testing.T) {
-	if v := classifySyscallError(io.EOF); v != "" {
+	t.Run("for a non-syscall error", func(t *testing.T) {
-		t.Fatalf("expected empty string, got '%s'", v)
+		if v := classifySyscallError(io.EOF); v != "" {
-	}
+			t.Fatalf("expected empty string, got '%s'", v)
-	if v := classifySyscallError(ECANCELED); v != FailureOperationCanceled {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureOperationCanceled, v)
+	})
-	}
+
-	if v := classifySyscallError(ECONNREFUSED); v != FailureConnectionRefused {
+	t.Run("for ECANCELED", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureConnectionRefused, v)
+		if v := classifySyscallError(ECANCELED); v != FailureOperationCanceled {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureOperationCanceled, v)
-	if v := classifySyscallError(ECONNRESET); v != FailureConnectionReset {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureConnectionReset, v)
+	})
-	}
+
-	if v := classifySyscallError(EHOSTUNREACH); v != FailureHostUnreachable {
+	t.Run("for ECONNREFUSED", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureHostUnreachable, v)
+		if v := classifySyscallError(ECONNREFUSED); v != FailureConnectionRefused {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureConnectionRefused, v)
-	if v := classifySyscallError(ETIMEDOUT); v != FailureTimedOut {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureTimedOut, v)
+	})
-	}
+
-	if v := classifySyscallError(EAFNOSUPPORT); v != FailureAddressFamilyNotSupported {
+	t.Run("for ECONNRESET", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureAddressFamilyNotSupported, v)
+		if v := classifySyscallError(ECONNRESET); v != FailureConnectionReset {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureConnectionReset, v)
-	if v := classifySyscallError(EADDRINUSE); v != FailureAddressInUse {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureAddressInUse, v)
+	})
-	}
+
-	if v := classifySyscallError(EADDRNOTAVAIL); v != FailureAddressNotAvailable {
+	t.Run("for EHOSTUNREACH", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureAddressNotAvailable, v)
+		if v := classifySyscallError(EHOSTUNREACH); v != FailureHostUnreachable {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureHostUnreachable, v)
-	if v := classifySyscallError(EISCONN); v != FailureAlreadyConnected {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureAlreadyConnected, v)
+	})
-	}
+
-	if v := classifySyscallError(EFAULT); v != FailureBadAddress {
+	t.Run("for ETIMEDOUT", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureBadAddress, v)
+		if v := classifySyscallError(ETIMEDOUT); v != FailureTimedOut {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureTimedOut, v)
-	if v := classifySyscallError(EBADF); v != FailureBadFileDescriptor {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureBadFileDescriptor, v)
+	})
-	}
+
-	if v := classifySyscallError(ECONNABORTED); v != FailureConnectionAborted {
+	t.Run("for EAFNOSUPPORT", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureConnectionAborted, v)
+		if v := classifySyscallError(EAFNOSUPPORT); v != FailureAddressFamilyNotSupported {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureAddressFamilyNotSupported, v)
-	if v := classifySyscallError(EALREADY); v != FailureConnectionAlreadyInProgress {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureConnectionAlreadyInProgress, v)
+	})
-	}
+
-	if v := classifySyscallError(EDESTADDRREQ); v != FailureDestinationAddressRequired {
+	t.Run("for EADDRINUSE", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureDestinationAddressRequired, v)
+		if v := classifySyscallError(EADDRINUSE); v != FailureAddressInUse {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureAddressInUse, v)
-	if v := classifySyscallError(EINTR); v != FailureInterrupted {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureInterrupted, v)
+	})
-	}
+
-	if v := classifySyscallError(EINVAL); v != FailureInvalidArgument {
+	t.Run("for EADDRNOTAVAIL", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureInvalidArgument, v)
+		if v := classifySyscallError(EADDRNOTAVAIL); v != FailureAddressNotAvailable {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureAddressNotAvailable, v)
-	if v := classifySyscallError(EMSGSIZE); v != FailureMessageSize {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureMessageSize, v)
+	})
-	}
+
-	if v := classifySyscallError(ENETDOWN); v != FailureNetworkDown {
+	t.Run("for EISCONN", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureNetworkDown, v)
+		if v := classifySyscallError(EISCONN); v != FailureAlreadyConnected {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureAlreadyConnected, v)
-	if v := classifySyscallError(ENETRESET); v != FailureNetworkReset {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureNetworkReset, v)
+	})
-	}
+
-	if v := classifySyscallError(ENETUNREACH); v != FailureNetworkUnreachable {
+	t.Run("for EFAULT", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureNetworkUnreachable, v)
+		if v := classifySyscallError(EFAULT); v != FailureBadAddress {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureBadAddress, v)
-	if v := classifySyscallError(ENOBUFS); v != FailureNoBufferSpace {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureNoBufferSpace, v)
+	})
-	}
+
-	if v := classifySyscallError(ENOPROTOOPT); v != FailureNoProtocolOption {
+	t.Run("for EBADF", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureNoProtocolOption, v)
+		if v := classifySyscallError(EBADF); v != FailureBadFileDescriptor {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureBadFileDescriptor, v)
-	if v := classifySyscallError(ENOTSOCK); v != FailureNotASocket {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureNotASocket, v)
+	})
-	}
+
-	if v := classifySyscallError(ENOTCONN); v != FailureNotConnected {
+	t.Run("for ECONNABORTED", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureNotConnected, v)
+		if v := classifySyscallError(ECONNABORTED); v != FailureConnectionAborted {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureConnectionAborted, v)
-	if v := classifySyscallError(EWOULDBLOCK); v != FailureOperationWouldBlock {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureOperationWouldBlock, v)
+	})
-	}
+
-	if v := classifySyscallError(EACCES); v != FailurePermissionDenied {
+	t.Run("for EALREADY", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailurePermissionDenied, v)
+		if v := classifySyscallError(EALREADY); v != FailureConnectionAlreadyInProgress {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureConnectionAlreadyInProgress, v)
-	if v := classifySyscallError(EPROTONOSUPPORT); v != FailureProtocolNotSupported {
+		}
-		t.Fatalf("expected '%s', got '%s'", FailureProtocolNotSupported, v)
+	})
-	}
+
-	if v := classifySyscallError(EPROTOTYPE); v != FailureWrongProtocolType {
+	t.Run("for EDESTADDRREQ", func(t *testing.T) {
-		t.Fatalf("expected '%s', got '%s'", FailureWrongProtocolType, v)
+		if v := classifySyscallError(EDESTADDRREQ); v != FailureDestinationAddressRequired {
-	}
+			t.Fatalf("expected '%s', got '%s'", FailureDestinationAddressRequired, v)
-	if v := classifySyscallError(syscall.Errno(0)); v != "" {
+		}
-		t.Fatalf("expected empty string, got '%s'", v)
+	})
-	}
+
+	t.Run("for EINTR", func(t *testing.T) {
+		if v := classifySyscallError(EINTR); v != FailureInterrupted {
+			t.Fatalf("expected '%s', got '%s'", FailureInterrupted, v)
+		}
+	})
+
+	t.Run("for EINVAL", func(t *testing.T) {
+		if v := classifySyscallError(EINVAL); v != FailureInvalidArgument {
+			t.Fatalf("expected '%s', got '%s'", FailureInvalidArgument, v)
+		}
+	})
+
+	t.Run("for EMSGSIZE", func(t *testing.T) {
+		if v := classifySyscallError(EMSGSIZE); v != FailureMessageSize {
+			t.Fatalf("expected '%s', got '%s'", FailureMessageSize, v)
+		}
+	})
+
+	t.Run("for ENETDOWN", func(t *testing.T) {
+		if v := classifySyscallError(ENETDOWN); v != FailureNetworkDown {
+			t.Fatalf("expected '%s', got '%s'", FailureNetworkDown, v)
+		}
+	})
+
+	t.Run("for ENETRESET", func(t *testing.T) {
+		if v := classifySyscallError(ENETRESET); v != FailureNetworkReset {
+			t.Fatalf("expected '%s', got '%s'", FailureNetworkReset, v)
+		}
+	})
+
+	t.Run("for ENETUNREACH", func(t *testing.T) {
+		if v := classifySyscallError(ENETUNREACH); v != FailureNetworkUnreachable {
+			t.Fatalf("expected '%s', got '%s'", FailureNetworkUnreachable, v)
+		}
+	})
+
+	t.Run("for ENOBUFS", func(t *testing.T) {
+		if v := classifySyscallError(ENOBUFS); v != FailureNoBufferSpace {
+			t.Fatalf("expected '%s', got '%s'", FailureNoBufferSpace, v)
+		}
+	})
+
+	t.Run("for ENOPROTOOPT", func(t *testing.T) {
+		if v := classifySyscallError(ENOPROTOOPT); v != FailureNoProtocolOption {
+			t.Fatalf("expected '%s', got '%s'", FailureNoProtocolOption, v)
+		}
+	})
+
+	t.Run("for ENOTSOCK", func(t *testing.T) {
+		if v := classifySyscallError(ENOTSOCK); v != FailureNotASocket {
+			t.Fatalf("expected '%s', got '%s'", FailureNotASocket, v)
+		}
+	})
+
+	t.Run("for ENOTCONN", func(t *testing.T) {
+		if v := classifySyscallError(ENOTCONN); v != FailureNotConnected {
+			t.Fatalf("expected '%s', got '%s'", FailureNotConnected, v)
+		}
+	})
+
+	t.Run("for EWOULDBLOCK", func(t *testing.T) {
+		if v := classifySyscallError(EWOULDBLOCK); v != FailureOperationWouldBlock {
+			t.Fatalf("expected '%s', got '%s'", FailureOperationWouldBlock, v)
+		}
+	})
+
+	t.Run("for EACCES", func(t *testing.T) {
+		if v := classifySyscallError(EACCES); v != FailurePermissionDenied {
+			t.Fatalf("expected '%s', got '%s'", FailurePermissionDenied, v)
+		}
+	})
+
+	t.Run("for EPROTONOSUPPORT", func(t *testing.T) {
+		if v := classifySyscallError(EPROTONOSUPPORT); v != FailureProtocolNotSupported {
+			t.Fatalf("expected '%s', got '%s'", FailureProtocolNotSupported, v)
+		}
+	})
+
+	t.Run("for EPROTOTYPE", func(t *testing.T) {
+		if v := classifySyscallError(EPROTOTYPE); v != FailureWrongProtocolType {
+			t.Fatalf("expected '%s', got '%s'", FailureWrongProtocolType, v)
+		}
+	})
+
+	t.Run("for the zero errno value", func(t *testing.T) {
+		if v := classifySyscallError(syscall.Errno(0)); v != "" {
+			t.Fatalf("expected empty string, got '%s'", v)
+		}
+	})
 }

internal/netxlite/errorsx/errno_unix.go

@@ -1,5 +1,5 @@
 // Code generated by go generate; DO NOT EDIT.
-// Generated: 2021-09-07 16:43:08.35751 +0200 CEST m=+0.000202959
+// Generated: 2021-09-07 20:26:06.370246 +0200 CEST m=+0.001036417
 
 package errorsx
 

internal/netxlite/errorsx/errno_windows.go

@@ -1,5 +1,5 @@
 // Code generated by go generate; DO NOT EDIT.
-// Generated: 2021-09-07 16:43:08.436584 +0200 CEST m=+0.079277834
+// Generated: 2021-09-07 20:26:06.478577 +0200 CEST m=+0.109369751
 
 package errorsx
 

internal/netxlite/errorsx/errwrapper.go

@@ -2,17 +2,28 @@ package errorsx
 
 // ErrWrapper is our error wrapper for Go errors. The key objective of
 // this structure is to properly set Failure, which is also returned by
-// the Error() method, so be one of the OONI defined strings.
+// the Error() method, to be one of the OONI failure strings.
+//
+// OONI failure strings are defined in the github.com/ooni/spec repo
+// at https://github.com/ooni/spec/blob/master/data-formats/df-007-errors.md.
 type ErrWrapper struct {
 	// Failure is the OONI failure string. The failure strings are
 	// loosely backward compatible with Measurement Kit.
 	//
 	// This is either one of the FailureXXX strings or any other
-	// string like `unknown_failure ...`. The latter represents an
+	// string like `unknown_failure: ...`. The latter represents an
 	// error that we have not yet mapped to a failure.
 	Failure string
 
-	// Operation is the operation that failed. If possible, it
+	// 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:
 	//
 	// - ResolveOperation: resolving a domain name failed

internal/netxlite/errorsx/errwrapper_test.go

@@ -6,19 +6,21 @@ import (
 	"testing"
 )
 
-func TestErrWrapperError(t *testing.T) {
+func TestErrWrapper(t *testing.T) {
-	err := &ErrWrapper{Failure: FailureDNSNXDOMAINError}
+	t.Run("Error", func(t *testing.T) {
-	if err.Error() != FailureDNSNXDOMAINError {
+		err := &ErrWrapper{Failure: FailureDNSNXDOMAINError}
-		t.Fatal("invalid return value")
+		if err.Error() != FailureDNSNXDOMAINError {
-	}
+			t.Fatal("invalid return value")
-}
+		}
+	})
 
-func TestErrWrapperUnwrap(t *testing.T) {
+	t.Run("Unwrap", func(t *testing.T) {
-	err := &ErrWrapper{
+		err := &ErrWrapper{
-		Failure:    FailureEOFError,
+			Failure:    FailureEOFError,
-		WrappedErr: io.EOF,
+			WrappedErr: io.EOF,
-	}
+		}
-	if !errors.Is(err, io.EOF) {
+		if !errors.Is(err, io.EOF) {
-		t.Fatal("cannot unwrap error")
+			t.Fatal("cannot unwrap error")
-	}
+		}
+	})
 }

internal/netxlite/errorsx/internal/generrno/main.go

@@ -234,25 +234,32 @@ func writeGenericTestFile() {
 	fileWrite(filep, "\t\"testing\"\n")
 	fileWrite(filep, ")\n\n")
 
-	fileWrite(filep, "func TestToSyscallErr(t *testing.T) {\n")
+	fileWrite(filep, "func TestClassifySyscallError(t *testing.T) {\n")
-	fileWrite(filep, "\tif v := classifySyscallError(io.EOF); v != \"\" {\n")
+	fileWrite(filep, "\tt.Run(\"for a non-syscall error\", func (t *testing.T) {\n")
-	fileWrite(filep, "\t\tt.Fatalf(\"expected empty string, got '%s'\", v)\n")
+	fileWrite(filep, "\t\tif v := classifySyscallError(io.EOF); v != \"\" {\n")
-	fileWrite(filep, "\t}\n")
+	fileWrite(filep, "\t\t\tt.Fatalf(\"expected empty string, got '%s'\", v)\n")
+	fileWrite(filep, "\t\t}\n")
+	fileWrite(filep, "\t})\n\n")
 
 	for _, spec := range Specs {
 		if !spec.IsSystemError() {
 			continue
 		}
-		filePrintf(filep, "\tif v := classifySyscallError(%s); v != %s {\n",
+		filePrintf(filep, "\tt.Run(\"for %s\", func (t *testing.T) {\n",
+			spec.AsErrnoName())
+		filePrintf(filep, "\t\tif v := classifySyscallError(%s); v != %s {\n",
 			spec.AsErrnoName(), spec.AsFailureVar())
-		filePrintf(filep, "\t\tt.Fatalf(\"expected '%%s', got '%%s'\", %s, v)\n",
+		filePrintf(filep, "\t\t\tt.Fatalf(\"expected '%%s', got '%%s'\", %s, v)\n",
 			spec.AsFailureVar())
-		fileWrite(filep, "\t}\n")
+		fileWrite(filep, "\t\t}\n")
+		fileWrite(filep, "\t})\n\n")
 	}
 
-	fileWrite(filep, "\tif v := classifySyscallError(syscall.Errno(0)); v != \"\" {\n")
+	fileWrite(filep, "\tt.Run(\"for the zero errno value\", func (t *testing.T) {\n")
-	fileWrite(filep, "\t\tt.Fatalf(\"expected empty string, got '%s'\", v)\n")
+	fileWrite(filep, "\t\tif v := classifySyscallError(syscall.Errno(0)); v != \"\" {\n")
-	fileWrite(filep, "\t}\n")
+	fileWrite(filep, "\t\t\tt.Fatalf(\"expected empty string, got '%s'\", v)\n")
+	fileWrite(filep, "\t\t}\n")
+	fileWrite(filep, "\t})\n")
 	fileWrite(filep, "}\n")
 
 	fileClose(filep)

internal/netxlite/errorsx/operations.go

@@ -1,6 +1,7 @@
 package errorsx
 
-// Operations that we measure.
+// Operations that we measure. They are the possibly values of
+// the ErrWrapper.Operation field.
 const (
 	// ResolveOperation is the operation where we resolve a domain name.
 	ResolveOperation = "resolve"