From 8174d88baceadf04ee236f771d2070ed78459d41 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Tue, 7 Sep 2021 17:23:24 +0200
Subject: [PATCH] refactor(i/errorsx): always use the same error reporting
 pattern (#478)

For consistency and also because the SafeErrorWrapperBuilder seems
to be the building pattern than the original code assumed.

New code should not use it, but I'd rather keep legacy code consistent
formally and with its own original assumptions.

In particular, it matters that SafeErrorWrapperBuilder assigns the
most relevant operation that failed. We were not doing that when we
were manually creating a new ErrWrapper.

Part of https://github.com/ooni/probe/issues/1591
---
 internal/errorsx/dialer.go | 32 ++++++++++++++++----------------
 internal/errorsx/quic.go   | 11 +++++------
 2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/internal/errorsx/dialer.go b/internal/errorsx/dialer.go
index bca0c9a..0a49fc6 100644
--- a/internal/errorsx/dialer.go
+++ b/internal/errorsx/dialer.go
@@ -24,11 +24,11 @@ type ErrorWrapperDialer struct {
 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, &errorsx.ErrWrapper{
-			Failure:    errorsx.ClassifyGenericError(err),
+		return nil, SafeErrWrapperBuilder{
+			Classifier: errorsx.ClassifyGenericError,
 			Operation:  errorsx.ConnectOperation,
-			WrappedErr: err,
-		}
+			Error:      err,
+		}.MaybeBuild()
 	}
 	return &errorWrapperConn{Conn: conn}, nil
 }
@@ -43,11 +43,11 @@ type errorWrapperConn struct {
 func (c *errorWrapperConn) Read(b []byte) (int, error) {
 	count, err := c.Conn.Read(b)
 	if err != nil {
-		return 0, &errorsx.ErrWrapper{
-			Failure:    errorsx.ClassifyGenericError(err),
+		return 0, SafeErrWrapperBuilder{
+			Classifier: errorsx.ClassifyGenericError,
 			Operation:  errorsx.ReadOperation,
-			WrappedErr: err,
-		}
+			Error:      err,
+		}.MaybeBuild()
 	}
 	return count, nil
 }
@@ -56,11 +56,11 @@ func (c *errorWrapperConn) Read(b []byte) (int, error) {
 func (c *errorWrapperConn) Write(b []byte) (int, error) {
 	count, err := c.Conn.Write(b)
 	if err != nil {
-		return 0, &errorsx.ErrWrapper{
-			Failure:    errorsx.ClassifyGenericError(err),
+		return 0, SafeErrWrapperBuilder{
+			Classifier: errorsx.ClassifyGenericError,
 			Operation:  errorsx.WriteOperation,
-			WrappedErr: err,
-		}
+			Error:      err,
+		}.MaybeBuild()
 	}
 	return count, nil
 }
@@ -69,11 +69,11 @@ func (c *errorWrapperConn) Write(b []byte) (int, error) {
 func (c *errorWrapperConn) Close() error {
 	err := c.Conn.Close()
 	if err != nil {
-		return &errorsx.ErrWrapper{
-			Failure:    errorsx.ClassifyGenericError(err),
+		return SafeErrWrapperBuilder{
+			Classifier: errorsx.ClassifyGenericError,
 			Operation:  errorsx.CloseOperation,
-			WrappedErr: err,
-		}
+			Error:      err,
+		}.MaybeBuild()
 	}
 	return nil
 }
diff --git a/internal/errorsx/quic.go b/internal/errorsx/quic.go
index f94149f..fb532a3 100644
--- a/internal/errorsx/quic.go
+++ b/internal/errorsx/quic.go
@@ -87,13 +87,12 @@ 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)
-	err = SafeErrWrapperBuilder{
-		Classifier: errorsx.ClassifyQUICHandshakeError,
-		Error:      err,
-		Operation:  errorsx.QUICHandshakeOperation,
-	}.MaybeBuild()
 	if err != nil {
-		return nil, err
+		return nil, SafeErrWrapperBuilder{
+			Classifier: errorsx.ClassifyQUICHandshakeError,
+			Error:      err,
+			Operation:  errorsx.QUICHandshakeOperation,
+		}.MaybeBuild()
 	}
 	return sess, nil
 }