From c1b06a2d09b323a2be2f792811d5a7c09b838a89 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Sun, 15 May 2022 19:25:27 +0200
Subject: [PATCH] fix(netxlite): prefer composition over embedding (#733)

This diff has been extracted and adapted from https://github.com/bassosimone/websteps-illustrated/commit/8848c8c516a40663c2718b1aff271884a116a147

The reason to prefer composition over embedding is that we want the
build to break if we add new methods to interfaces we define. If the build
does not break, we may forget about wrapping methods we should
actually be wrapping. I noticed this issue inside netxlite when I was working
on websteps-illustrated and I added support for NS and PTR queries.

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

While there, perform comprehensive netxlite code review
and apply minor changes and improve the docs.
---
 internal/netxlite/bogon.go            | 13 +++--
 internal/netxlite/classify.go         | 16 +++---
 internal/netxlite/dialer.go           | 34 ++++++++-----
 internal/netxlite/dialer_test.go      |  4 +-
 internal/netxlite/dnsdecoder.go       |  4 ++
 internal/netxlite/dnsencoder.go       |  4 ++
 internal/netxlite/dnsoverhttps.go     |  4 ++
 internal/netxlite/dnsovertcp.go       |  4 ++
 internal/netxlite/dnsoverudp.go       |  4 ++
 internal/netxlite/dnstransport.go     |  1 -
 internal/netxlite/doc.go              |  2 +-
 internal/netxlite/errwrapper.go       | 14 +++---
 internal/netxlite/errwrapper_test.go  | 12 ++---
 internal/netxlite/http.go             | 72 +++++++++++++++++++++++----
 internal/netxlite/http3.go            |  4 ++
 internal/netxlite/http_test.go        | 24 +++++++--
 internal/netxlite/iox.go              |  4 ++
 internal/netxlite/iox_test.go         |  4 +-
 internal/netxlite/legacy.go           |  4 ++
 internal/netxlite/operations.go       |  4 ++
 internal/netxlite/parallelresolver.go |  2 +-
 internal/netxlite/quic.go             | 30 +++++++----
 internal/netxlite/quirks.go           | 10 ++--
 internal/netxlite/quirks_test.go      |  4 +-
 internal/netxlite/resolver.go         | 70 +++++++++++++++++++++++---
 internal/netxlite/resolver_test.go    | 24 +++++++++
 internal/netxlite/serialresolver.go   | 14 ++++--
 internal/netxlite/tls.go              | 25 +++++++---
 internal/netxlite/tls_test.go         |  1 +
 internal/netxlite/tproxy.go           |  4 ++
 internal/netxlite/utls.go             |  4 ++
 31 files changed, 328 insertions(+), 92 deletions(-)
 delete mode 100644 internal/netxlite/dnstransport.go

diff --git a/internal/netxlite/bogon.go b/internal/netxlite/bogon.go
index 47a1c27..cbac027 100644
--- a/internal/netxlite/bogon.go
+++ b/internal/netxlite/bogon.go
@@ -18,7 +18,7 @@ import (
 // function a non-IP address causes it to return true.
 func IsBogon(address string) bool {
 	ip := net.ParseIP(address)
-	return ip == nil || isPrivate(address, ip)
+	return ip == nil || isBogon(address, ip)
 }
 
 // IsLoopback returns whether an IP address is loopback. Passing to this
@@ -33,7 +33,7 @@ var (
 	bogons6 []*net.IPNet
 )
 
-func expandbogons(cidrs []string) (out []*net.IPNet) {
+func expandBogons(cidrs []string) (out []*net.IPNet) {
 	for _, cidr := range cidrs {
 		_, block, err := net.ParseCIDR(cidr)
 		runtimex.PanicOnError(err, "net.ParseCIDR failed")
@@ -43,7 +43,7 @@ func expandbogons(cidrs []string) (out []*net.IPNet) {
 }
 
 func init() {
-	bogons4 = append(bogons4, expandbogons([]string{
+	bogons4 = append(bogons4, expandBogons([]string{
 		//
 		// List extracted from https://ipinfo.io/bogon
 		//
@@ -64,7 +64,7 @@ func init() {
 		"240.0.0.0/4",        // Reserved for future use
 		"255.255.255.255/32", // Limited broadcast
 	})...)
-	bogons6 = append(bogons6, expandbogons([]string{
+	bogons6 = append(bogons6, expandBogons([]string{
 		//
 		// List extracted from https://ipinfo.io/bogon
 		//
@@ -110,7 +110,10 @@ func init() {
 	})...)
 }
 
-func isPrivate(address string, ip net.IP) bool {
+// isBogon implements IsBogon
+func isBogon(address string, ip net.IP) bool {
+	// TODO(bassosimone): the following check is probably redundant given that these
+	// three checks are already included into the list of bogons.
 	if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
 		return true
 	}
diff --git a/internal/netxlite/classify.go b/internal/netxlite/classify.go
index 6bb3d21..254ff61 100644
--- a/internal/netxlite/classify.go
+++ b/internal/netxlite/classify.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Mapping Go errors to OONI errors
+//
+
 import (
 	"context"
 	"crypto/x509"
@@ -38,8 +42,7 @@ func classifyGenericError(err error) string {
 	// The list returned here matches the values used by MK unless
 	// explicitly noted otherwise with a comment.
 
-	// QUIRK: we cannot remove this check as long as this function
-	// is exported and used independently from NewErrWrapper.
+	// Robustness: handle the case where we're passed a wrapped error.
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
 		return errwrapper.Error() // we've already wrapped it
@@ -146,8 +149,7 @@ const (
 // and returns to the caller its return value.
 func classifyQUICHandshakeError(err error) string {
 
-	// QUIRK: we cannot remove this check as long as this function
-	// is exported and used independently from NewErrWrapper.
+	// Robustness: handle the case where we're passed a wrapped error.
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
 		return errwrapper.Error() // we've already wrapped it
@@ -269,8 +271,7 @@ var (
 // returns to the caller its return value.
 func classifyResolverError(err error) string {
 
-	// QUIRK: we cannot remove this check as long as this function
-	// is exported and used independently from NewErrWrapper.
+	// Robustness: handle the case where we're passed a wrapped error.
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
 		return errwrapper.Error() // we've already wrapped it
@@ -303,8 +304,7 @@ func classifyResolverError(err error) string {
 // returns to the caller its return value.
 func classifyTLSHandshakeError(err error) string {
 
-	// QUIRK: we cannot remove this check as long as this function
-	// is exported and used independently from NewErrWrapper.
+	// Robustness: handle the case where we're passed a wrapped error.
 	var errwrapper *ErrWrapper
 	if errors.As(err, &errwrapper) {
 		return errwrapper.Error() // we've already wrapped it
diff --git a/internal/netxlite/dialer.go b/internal/netxlite/dialer.go
index 4c1f754..43cade5 100644
--- a/internal/netxlite/dialer.go
+++ b/internal/netxlite/dialer.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Code for dialing TCP or UDP net.Conn-like connections
+//
+
 import (
 	"context"
 	"errors"
@@ -96,8 +100,8 @@ func (d *dialerSystem) CloseIdleConnections() {
 
 // dialerResolver combines dialing with domain name resolution.
 type dialerResolver struct {
-	model.Dialer
-	model.Resolver
+	Dialer   model.Dialer
+	Resolver model.Resolver
 }
 
 var _ model.Dialer = &dialerResolver{}
@@ -144,10 +148,10 @@ func (d *dialerResolver) CloseIdleConnections() {
 // dialerLogger is a Dialer with logging.
 type dialerLogger struct {
 	// Dialer is the underlying dialer.
-	model.Dialer
+	Dialer model.Dialer
 
-	// Logger is the underlying logger.
-	model.DebugLogger
+	// DebugLogger is the underlying logger.
+	DebugLogger model.DebugLogger
 
 	// operationSuffix is appended to the operation name.
 	//
@@ -194,15 +198,15 @@ func NewSingleUseDialer(conn net.Conn) model.Dialer {
 
 // dialerSingleUse is the Dialer returned by NewSingleDialer.
 type dialerSingleUse struct {
-	sync.Mutex
+	mu   sync.Mutex
 	conn net.Conn
 }
 
 var _ model.Dialer = &dialerSingleUse{}
 
 func (s *dialerSingleUse) DialContext(ctx context.Context, network string, addr string) (net.Conn, error) {
-	defer s.Unlock()
-	s.Lock()
+	defer s.mu.Unlock()
+	s.mu.Lock()
 	if s.conn == nil {
 		return nil, ErrNoConnReuse
 	}
@@ -218,7 +222,7 @@ func (s *dialerSingleUse) CloseIdleConnections() {
 // dialerErrWrapper is a dialer that performs error wrapping. The connection
 // returned by the DialContext function will also perform error wrapping.
 type dialerErrWrapper struct {
-	model.Dialer
+	Dialer model.Dialer
 }
 
 var _ model.Dialer = &dialerErrWrapper{}
@@ -226,11 +230,15 @@ 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
 }
 
+func (d *dialerErrWrapper) CloseIdleConnections() {
+	d.Dialer.CloseIdleConnections()
+}
+
 // dialerErrWrapperConn is a net.Conn that performs error wrapping.
 type dialerErrWrapperConn struct {
 	net.Conn
@@ -241,7 +249,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 +257,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 +265,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 646e844..f421a1c 100644
--- a/internal/netxlite/dialer_test.go
+++ b/internal/netxlite/dialer_test.go
@@ -253,7 +253,7 @@ func TestDialerResolver(t *testing.T) {
 			mu := &sync.Mutex{}
 			errorsList := []error{
 				errors.New("a mocked error"),
-				NewErrWrapper(
+				newErrWrapper(
 					classifyGenericError,
 					CloseOperation,
 					io.EOF,
@@ -295,7 +295,7 @@ func TestDialerResolver(t *testing.T) {
 			mu := &sync.Mutex{}
 			errorsList := []error{
 				errors.New("a mocked error"),
-				NewErrWrapper(
+				newErrWrapper(
 					classifyGenericError,
 					CloseOperation,
 					errors.New("antani"),
diff --git a/internal/netxlite/dnsdecoder.go b/internal/netxlite/dnsdecoder.go
index 22ef9fb..a557101 100644
--- a/internal/netxlite/dnsdecoder.go
+++ b/internal/netxlite/dnsdecoder.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Decode byte arrays to DNS messages
+//
+
 import (
 	"errors"
 
diff --git a/internal/netxlite/dnsencoder.go b/internal/netxlite/dnsencoder.go
index f5394be..91e3ba1 100644
--- a/internal/netxlite/dnsencoder.go
+++ b/internal/netxlite/dnsencoder.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Encode DNS queries to byte arrays
+//
+
 import (
 	"github.com/miekg/dns"
 	"github.com/ooni/probe-cli/v3/internal/model"
diff --git a/internal/netxlite/dnsoverhttps.go b/internal/netxlite/dnsoverhttps.go
index 8c9cf49..7d1215a 100644
--- a/internal/netxlite/dnsoverhttps.go
+++ b/internal/netxlite/dnsoverhttps.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// DNS-over-HTTPS transport
+//
+
 import (
 	"bytes"
 	"context"
diff --git a/internal/netxlite/dnsovertcp.go b/internal/netxlite/dnsovertcp.go
index eeafe36..190b946 100644
--- a/internal/netxlite/dnsovertcp.go
+++ b/internal/netxlite/dnsovertcp.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// DNS-over-{TCP,TLS} transport
+//
+
 import (
 	"context"
 	"errors"
diff --git a/internal/netxlite/dnsoverudp.go b/internal/netxlite/dnsoverudp.go
index 8b336b0..4a79271 100644
--- a/internal/netxlite/dnsoverudp.go
+++ b/internal/netxlite/dnsoverudp.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// DNS-over-UDP transport
+//
+
 import (
 	"context"
 	"time"
diff --git a/internal/netxlite/dnstransport.go b/internal/netxlite/dnstransport.go
deleted file mode 100644
index 33539e6..0000000
--- a/internal/netxlite/dnstransport.go
+++ /dev/null
@@ -1 +0,0 @@
-package netxlite
diff --git a/internal/netxlite/doc.go b/internal/netxlite/doc.go
index c5d01ed..7a8f9cc 100644
--- a/internal/netxlite/doc.go
+++ b/internal/netxlite/doc.go
@@ -3,7 +3,7 @@
 // This package is the basic networking building block that you
 // should be using every time you need networking.
 //
-// It implements interfaces defined in the internal/model package.
+// It implements interfaces defined in internal/model/netx.go.
 //
 // You should consider checking the tutorial explaining how to use this package
 // for network measurements: https://github.com/ooni/probe-cli/tree/master/internal/tutorial/netxlite.
diff --git a/internal/netxlite/errwrapper.go b/internal/netxlite/errwrapper.go
index 121ea2a..9af52c1 100644
--- a/internal/netxlite/errwrapper.go
+++ b/internal/netxlite/errwrapper.go
@@ -66,12 +66,12 @@ func (e *ErrWrapper) MarshalJSON() ([]byte, error) {
 	return json.Marshal(e.Failure)
 }
 
-// Classifier is the type of the function that maps a Go error
+// classifier is the type of the function that maps a Go error
 // to a OONI failure string defined at
 // https://github.com/ooni/spec/blob/master/data-formats/df-007-errors.md.
-type Classifier func(err error) string
+type classifier func(err error) string
 
-// NewErrWrapper creates a new ErrWrapper using the given
+// newErrWrapper creates a new ErrWrapper using the given
 // classifier, operation name, and underlying error.
 //
 // This function panics if classifier is nil, or operation
@@ -81,7 +81,7 @@ type Classifier func(err error) string
 // error wrapper will use the same classification string and
 // will determine whether to keep the major operation as documented
 // in the ErrWrapper.Operation documentation.
-func NewErrWrapper(c Classifier, op string, err error) *ErrWrapper {
+func newErrWrapper(c classifier, op string, err error) *ErrWrapper {
 	var wrapper *ErrWrapper
 	if errors.As(err, &wrapper) {
 		return &ErrWrapper{
@@ -107,13 +107,15 @@ func NewErrWrapper(c Classifier, op string, err error) *ErrWrapper {
 }
 
 // NewTopLevelGenericErrWrapper wraps an error occurring at top
-// level using ClassifyGenericError as classifier.
+// level using a generic classifier as classifier. This is the
+// function you should call when you suspect a given error hasn't
+// already been wrapped. This function panics if err is nil.
 //
 // If the err argument has already been classified, the returned
 // 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 {
diff --git a/internal/netxlite/errwrapper_test.go b/internal/netxlite/errwrapper_test.go
index e4066d3..2960dcc 100644
--- a/internal/netxlite/errwrapper_test.go
+++ b/internal/netxlite/errwrapper_test.go
@@ -53,7 +53,7 @@ func TestNewErrWrapper(t *testing.T) {
 					recovered.Add(1)
 				}
 			}()
-			NewErrWrapper(nil, CloseOperation, io.EOF)
+			newErrWrapper(nil, CloseOperation, io.EOF)
 		}()
 		if recovered.Load() != 1 {
 			t.Fatal("did not panic")
@@ -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")
 		}
@@ -104,10 +104,10 @@ func TestNewErrWrapper(t *testing.T) {
 	})
 
 	t.Run("when the underlying error is already a wrapped error", func(t *testing.T) {
-		ew := NewErrWrapper(classifySyscallError, ReadOperation, ECONNRESET)
+		ew := newErrWrapper(classifySyscallError, ReadOperation, ECONNRESET)
 		var err1 error = ew
 		err2 := fmt.Errorf("cannot read: %w", err1)
-		ew2 := NewErrWrapper(classifyGenericError, HTTPRoundTripOperation, err2)
+		ew2 := newErrWrapper(classifyGenericError, HTTPRoundTripOperation, err2)
 		if ew2.Failure != ew.Failure {
 			t.Fatal("not the same failure")
 		}
diff --git a/internal/netxlite/http.go b/internal/netxlite/http.go
index 1f3fe2c..d553fa7 100644
--- a/internal/netxlite/http.go
+++ b/internal/netxlite/http.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// HTTP/1.1 and HTTP2 code
+//
+
 import (
 	"context"
 	"errors"
@@ -13,9 +17,11 @@ import (
 
 // httpTransportErrWrapper is an HTTPTransport with error wrapping.
 type httpTransportErrWrapper struct {
-	model.HTTPTransport
+	HTTPTransport model.HTTPTransport
 }
 
+var _ model.HTTPTransport = &httpTransportErrWrapper{}
+
 func (txp *httpTransportErrWrapper) RoundTrip(req *http.Request) (*http.Response, error) {
 	resp, err := txp.HTTPTransport.RoundTrip(req)
 	if err != nil {
@@ -24,10 +30,18 @@ func (txp *httpTransportErrWrapper) RoundTrip(req *http.Request) (*http.Response
 	return resp, nil
 }
 
+func (txp *httpTransportErrWrapper) CloseIdleConnections() {
+	txp.HTTPTransport.CloseIdleConnections()
+}
+
+func (txp *httpTransportErrWrapper) Network() string {
+	return txp.HTTPTransport.Network()
+}
+
 // httpTransportLogger is an HTTPTransport with logging.
 type httpTransportLogger struct {
 	// HTTPTransport is the underlying HTTP transport.
-	model.HTTPTransport
+	HTTPTransport model.HTTPTransport
 
 	// Logger is the underlying logger.
 	Logger model.DebugLogger
@@ -62,12 +76,26 @@ func (txp *httpTransportLogger) CloseIdleConnections() {
 	txp.HTTPTransport.CloseIdleConnections()
 }
 
+func (txp *httpTransportLogger) Network() string {
+	return txp.HTTPTransport.Network()
+}
+
 // httpTransportConnectionsCloser is an HTTPTransport that
 // correctly forwards CloseIdleConnections calls.
 type httpTransportConnectionsCloser struct {
-	model.HTTPTransport
-	model.Dialer
-	model.TLSDialer
+	HTTPTransport model.HTTPTransport
+	Dialer        model.Dialer
+	TLSDialer     model.TLSDialer
+}
+
+var _ model.HTTPTransport = &httpTransportConnectionsCloser{}
+
+func (txp *httpTransportConnectionsCloser) RoundTrip(req *http.Request) (*http.Response, error) {
+	return txp.HTTPTransport.RoundTrip(req)
+}
+
+func (txp *httpTransportConnectionsCloser) Network() string {
+	return txp.HTTPTransport.Network()
 }
 
 // CloseIdleConnections forwards the CloseIdleConnections calls.
@@ -148,7 +176,17 @@ func NewOOHTTPBaseTransport(dialer model.Dialer, tlsDialer model.TLSDialer) mode
 
 // stdlibTransport wraps oohttp.StdlibTransport to add .Network()
 type stdlibTransport struct {
-	*oohttp.StdlibTransport
+	StdlibTransport *oohttp.StdlibTransport
+}
+
+var _ model.HTTPTransport = &stdlibTransport{}
+
+func (txp *stdlibTransport) CloseIdleConnections() {
+	txp.StdlibTransport.CloseIdleConnections()
+}
+
+func (txp *stdlibTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	return txp.StdlibTransport.RoundTrip(req)
 }
 
 // Network implements HTTPTransport.Network.
@@ -170,7 +208,13 @@ func WrapHTTPTransport(logger model.DebugLogger, txp model.HTTPTransport) model.
 // httpDialerWithReadTimeout enforces a read timeout for all HTTP
 // connections. See https://github.com/ooni/probe/issues/1609.
 type httpDialerWithReadTimeout struct {
-	model.Dialer
+	Dialer model.Dialer
+}
+
+var _ model.Dialer = &httpDialerWithReadTimeout{}
+
+func (d *httpDialerWithReadTimeout) CloseIdleConnections() {
+	d.Dialer.CloseIdleConnections()
 }
 
 // DialContext implements Dialer.DialContext.
@@ -186,7 +230,13 @@ func (d *httpDialerWithReadTimeout) DialContext(
 // httpTLSDialerWithReadTimeout enforces a read timeout for all HTTP
 // connections. See https://github.com/ooni/probe/issues/1609.
 type httpTLSDialerWithReadTimeout struct {
-	model.TLSDialer
+	TLSDialer model.TLSDialer
+}
+
+var _ model.TLSDialer = &httpTLSDialerWithReadTimeout{}
+
+func (d *httpTLSDialerWithReadTimeout) CloseIdleConnections() {
+	d.TLSDialer.CloseIdleConnections()
 }
 
 // ErrNotTLSConn occur when an interface accepts a net.Conn but
@@ -280,7 +330,7 @@ func WrapHTTPClient(clnt model.HTTPClient) model.HTTPClient {
 }
 
 type httpClientErrWrapper struct {
-	model.HTTPClient
+	HTTPClient model.HTTPClient
 }
 
 func (c *httpClientErrWrapper) Do(req *http.Request) (*http.Response, error) {
@@ -290,3 +340,7 @@ func (c *httpClientErrWrapper) Do(req *http.Request) (*http.Response, error) {
 	}
 	return resp, nil
 }
+
+func (c *httpClientErrWrapper) CloseIdleConnections() {
+	c.HTTPClient.CloseIdleConnections()
+}
diff --git a/internal/netxlite/http3.go b/internal/netxlite/http3.go
index 49cec12..0077f3a 100644
--- a/internal/netxlite/http3.go
+++ b/internal/netxlite/http3.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// HTTP3 code
+//
+
 import (
 	"crypto/tls"
 	"io"
diff --git a/internal/netxlite/http_test.go b/internal/netxlite/http_test.go
index e45cb01..873c6d1 100644
--- a/internal/netxlite/http_test.go
+++ b/internal/netxlite/http_test.go
@@ -252,19 +252,19 @@ func TestNewHTTPTransport(t *testing.T) {
 			t.Fatal("invalid tls dialer")
 		}
 		stdlib := connectionsCloser.HTTPTransport.(*stdlibTransport)
-		if !stdlib.Transport.ForceAttemptHTTP2 {
+		if !stdlib.StdlibTransport.ForceAttemptHTTP2 {
 			t.Fatal("invalid ForceAttemptHTTP2")
 		}
-		if !stdlib.Transport.DisableCompression {
+		if !stdlib.StdlibTransport.DisableCompression {
 			t.Fatal("invalid DisableCompression")
 		}
-		if stdlib.Transport.MaxConnsPerHost != 1 {
+		if stdlib.StdlibTransport.MaxConnsPerHost != 1 {
 			t.Fatal("invalid MaxConnPerHost")
 		}
-		if stdlib.Transport.DialTLSContext == nil {
+		if stdlib.StdlibTransport.DialTLSContext == nil {
 			t.Fatal("invalid DialTLSContext")
 		}
-		if stdlib.Transport.DialContext == nil {
+		if stdlib.StdlibTransport.DialContext == nil {
 			t.Fatal("invalid DialContext")
 		}
 	})
@@ -500,6 +500,20 @@ func TestHTTPClientErrWrapper(t *testing.T) {
 			}
 		})
 	})
+
+	t.Run("CloseIdleConnections", func(t *testing.T) {
+		var called bool
+		child := &mocks.HTTPClient{
+			MockCloseIdleConnections: func() {
+				called = true
+			},
+		}
+		clnt := &httpClientErrWrapper{child}
+		clnt.CloseIdleConnections()
+		if !called {
+			t.Fatal("not called")
+		}
+	})
 }
 
 func TestNewHTTPClientStdlib(t *testing.T) {
diff --git a/internal/netxlite/iox.go b/internal/netxlite/iox.go
index 1ae617a..04dde36 100644
--- a/internal/netxlite/iox.go
+++ b/internal/netxlite/iox.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// I/O extensions
+//
+
 import (
 	"context"
 	"errors"
diff --git a/internal/netxlite/iox_test.go b/internal/netxlite/iox_test.go
index ebf7e2b..268b8dd 100644
--- a/internal/netxlite/iox_test.go
+++ b/internal/netxlite/iox_test.go
@@ -41,7 +41,7 @@ func TestReadAllContext(t *testing.T) {
 				//
 				// Note: Returning a wrapped error to ensure we address
 				// https://github.com/ooni/probe/issues/1965
-				return len(b), NewErrWrapper(classifyGenericError,
+				return len(b), newErrWrapper(classifyGenericError,
 					ReadOperation, io.EOF)
 			},
 		}
@@ -171,7 +171,7 @@ func TestCopyContext(t *testing.T) {
 				//
 				// Note: Returning a wrapped error to ensure we address
 				// https://github.com/ooni/probe/issues/1965
-				return len(b), NewErrWrapper(classifyGenericError,
+				return len(b), newErrWrapper(classifyGenericError,
 					ReadOperation, io.EOF)
 			},
 		}
diff --git a/internal/netxlite/legacy.go b/internal/netxlite/legacy.go
index d213220..8d72f97 100644
--- a/internal/netxlite/legacy.go
+++ b/internal/netxlite/legacy.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Legacy code
+//
+
 // These vars export internal names to legacy ooni/probe-cli code.
 //
 // Deprecated: do not use these names in new code.
diff --git a/internal/netxlite/operations.go b/internal/netxlite/operations.go
index e675074..773659b 100644
--- a/internal/netxlite/operations.go
+++ b/internal/netxlite/operations.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Names of operations
+//
+
 // Operations that we measure. They are the possible values of
 // the ErrWrapper.Operation field.
 const (
diff --git a/internal/netxlite/parallelresolver.go b/internal/netxlite/parallelresolver.go
index bf0e2cc..4b80eb8 100644
--- a/internal/netxlite/parallelresolver.go
+++ b/internal/netxlite/parallelresolver.go
@@ -1,7 +1,7 @@
 package netxlite
 
 //
-// Parallel resolver implementation
+// Parallel DNS resolver implementation
 //
 
 import (
diff --git a/internal/netxlite/quic.go b/internal/netxlite/quic.go
index 86a44e6..8bfe027 100644
--- a/internal/netxlite/quic.go
+++ b/internal/netxlite/quic.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// QUIC implementation
+//
+
 import (
 	"context"
 	"crypto/tls"
@@ -340,7 +344,7 @@ func NewSingleUseQUICDialer(qconn quic.EarlyConnection) model.QUICDialer {
 
 // quicDialerSingleUse is the QUICDialer returned by NewSingleQUICDialer.
 type quicDialerSingleUse struct {
-	sync.Mutex
+	mu    sync.Mutex
 	qconn quic.EarlyConnection
 }
 
@@ -351,8 +355,8 @@ func (s *quicDialerSingleUse) DialContext(
 	ctx context.Context, network, addr string, tlsCfg *tls.Config,
 	cfg *quic.Config) (quic.EarlyConnection, error) {
 	var qconn quic.EarlyConnection
-	defer s.Unlock()
-	s.Lock()
+	defer s.mu.Unlock()
+	s.mu.Lock()
 	if s.qconn == nil {
 		return nil, ErrNoConnReuse
 	}
@@ -368,7 +372,7 @@ func (s *quicDialerSingleUse) CloseIdleConnections() {
 // quicListenerErrWrapper is a QUICListener that wraps errors.
 type quicListenerErrWrapper struct {
 	// QUICListener is the underlying listener.
-	model.QUICListener
+	QUICListener model.QUICListener
 }
 
 var _ model.QUICListener = &quicListenerErrWrapper{}
@@ -377,7 +381,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
 }
@@ -394,7 +398,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
 }
@@ -403,7 +407,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
 }
@@ -412,24 +416,30 @@ 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
 }
 
 // quicDialerErrWrapper is a dialer that performs quic err wrapping
 type quicDialerErrWrapper struct {
-	model.QUICDialer
+	QUICDialer model.QUICDialer
 }
 
+var _ model.QUICDialer = &quicDialerErrWrapper{}
+
 // DialContext implements ContextDialer.DialContext
 func (d *quicDialerErrWrapper) DialContext(
 	ctx context.Context, network string, host string,
 	tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlyConnection, error) {
 	qconn, err := d.QUICDialer.DialContext(ctx, network, host, tlsCfg, cfg)
 	if err != nil {
-		return nil, NewErrWrapper(
+		return nil, newErrWrapper(
 			classifyQUICHandshakeError, QUICHandshakeOperation, err)
 	}
 	return qconn, nil
 }
+
+func (d *quicDialerErrWrapper) CloseIdleConnections() {
+	d.QUICDialer.CloseIdleConnections()
+}
diff --git a/internal/netxlite/quirks.go b/internal/netxlite/quirks.go
index 72d5fa8..3989a33 100644
--- a/internal/netxlite/quirks.go
+++ b/internal/netxlite/quirks.go
@@ -1,15 +1,17 @@
 package netxlite
 
+//
+// This file contains weird stuff that we carried over from
+// the original netx implementation and that we cannot remove
+// or change without thinking about the consequences.
+//
+
 import (
 	"errors"
 	"net"
 	"strings"
 )
 
-// This file contains weird stuff that we carried over from
-// the original netx implementation and that we cannot remove
-// or change without thinking about the consequences.
-
 // See https://github.com/ooni/probe/issues/1985
 var errReduceErrorsEmptyList = errors.New("bug: reduceErrors given an empty list")
 
diff --git a/internal/netxlite/quirks_test.go b/internal/netxlite/quirks_test.go
index de62260..298d640 100644
--- a/internal/netxlite/quirks_test.go
+++ b/internal/netxlite/quirks_test.go
@@ -34,12 +34,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(
+		err2 := newErrWrapper(
 			classifyGenericError,
 			CloseOperation,
 			errors.New("antani"),
 		)
-		err3 := NewErrWrapper(
+		err3 := newErrWrapper(
 			classifyGenericError,
 			CloseOperation,
 			ECONNREFUSED,
diff --git a/internal/netxlite/resolver.go b/internal/netxlite/resolver.go
index c81b24f..9a201b5 100644
--- a/internal/netxlite/resolver.go
+++ b/internal/netxlite/resolver.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// DNS resolver
+//
+
 import (
 	"context"
 	"errors"
@@ -134,8 +138,8 @@ func (r *resolverSystem) LookupHTTPS(
 
 // resolverLogger is a resolver that emits events
 type resolverLogger struct {
-	model.Resolver
-	Logger model.DebugLogger
+	Resolver model.Resolver
+	Logger   model.DebugLogger
 }
 
 var _ model.Resolver = &resolverLogger{}
@@ -172,13 +176,27 @@ func (r *resolverLogger) LookupHTTPS(
 	return https, nil
 }
 
+func (r *resolverLogger) Address() string {
+	return r.Resolver.Address()
+}
+
+func (r *resolverLogger) Network() string {
+	return r.Resolver.Network()
+}
+
+func (r *resolverLogger) CloseIdleConnections() {
+	r.Resolver.CloseIdleConnections()
+}
+
 // resolverIDNA supports resolving Internationalized Domain Names.
 //
 // See RFC3492 for more information.
 type resolverIDNA struct {
-	model.Resolver
+	Resolver model.Resolver
 }
 
+var _ model.Resolver = &resolverIDNA{}
+
 func (r *resolverIDNA) LookupHost(ctx context.Context, hostname string) ([]string, error) {
 	host, err := idna.ToASCII(hostname)
 	if err != nil {
@@ -196,12 +214,26 @@ func (r *resolverIDNA) LookupHTTPS(
 	return r.Resolver.LookupHTTPS(ctx, host)
 }
 
+func (r *resolverIDNA) Network() string {
+	return r.Resolver.Network()
+}
+
+func (r *resolverIDNA) Address() string {
+	return r.Resolver.Address()
+}
+
+func (r *resolverIDNA) CloseIdleConnections() {
+	r.Resolver.CloseIdleConnections()
+}
+
 // resolverShortCircuitIPAddr recognizes when the input hostname is an
 // IP address and returns it immediately to the caller.
 type resolverShortCircuitIPAddr struct {
-	model.Resolver
+	Resolver model.Resolver
 }
 
+var _ model.Resolver = &resolverShortCircuitIPAddr{}
+
 func (r *resolverShortCircuitIPAddr) LookupHost(ctx context.Context, hostname string) ([]string, error) {
 	if net.ParseIP(hostname) != nil {
 		return []string{hostname}, nil
@@ -222,6 +254,18 @@ func (r *resolverShortCircuitIPAddr) LookupHTTPS(ctx context.Context, hostname s
 	return r.Resolver.LookupHTTPS(ctx, hostname)
 }
 
+func (r *resolverShortCircuitIPAddr) Network() string {
+	return r.Resolver.Network()
+}
+
+func (r *resolverShortCircuitIPAddr) Address() string {
+	return r.Resolver.Address()
+}
+
+func (r *resolverShortCircuitIPAddr) CloseIdleConnections() {
+	r.Resolver.CloseIdleConnections()
+}
+
 // IsIPv6 returns true if the given candidate is a valid IP address
 // representation and such representation is IPv6.
 func IsIPv6(candidate string) (bool, error) {
@@ -271,7 +315,7 @@ func (r *nullResolver) LookupHTTPS(
 
 // resolverErrWrapper is a Resolver that knows about wrapping errors.
 type resolverErrWrapper struct {
-	model.Resolver
+	Resolver model.Resolver
 }
 
 var _ model.Resolver = &resolverErrWrapper{}
@@ -279,7 +323,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
 }
@@ -288,7 +332,19 @@ 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
 }
+
+func (r *resolverErrWrapper) Network() string {
+	return r.Resolver.Network()
+}
+
+func (r *resolverErrWrapper) Address() string {
+	return r.Resolver.Address()
+}
+
+func (r *resolverErrWrapper) CloseIdleConnections() {
+	r.Resolver.CloseIdleConnections()
+}
diff --git a/internal/netxlite/resolver_test.go b/internal/netxlite/resolver_test.go
index e60ea26..6405aae 100644
--- a/internal/netxlite/resolver_test.go
+++ b/internal/netxlite/resolver_test.go
@@ -400,6 +400,30 @@ func TestResolverIDNA(t *testing.T) {
 			}
 		})
 	})
+
+	t.Run("Network", func(t *testing.T) {
+		child := &mocks.Resolver{
+			MockNetwork: func() string {
+				return "x"
+			},
+		}
+		r := &resolverIDNA{child}
+		if r.Network() != "x" {
+			t.Fatal("invalid network")
+		}
+	})
+
+	t.Run("Address", func(t *testing.T) {
+		child := &mocks.Resolver{
+			MockAddress: func() string {
+				return "x"
+			},
+		}
+		r := &resolverIDNA{child}
+		if r.Address() != "x" {
+			t.Fatal("invalid address")
+		}
+	})
 }
 
 func TestResolverShortCircuitIPAddr(t *testing.T) {
diff --git a/internal/netxlite/serialresolver.go b/internal/netxlite/serialresolver.go
index 66ab153..be1f4bb 100644
--- a/internal/netxlite/serialresolver.go
+++ b/internal/netxlite/serialresolver.go
@@ -1,7 +1,7 @@
 package netxlite
 
 //
-// Serial resolver implementation
+// Serial DNS resolver implementation
 //
 
 import (
@@ -20,7 +20,11 @@ import (
 //
 // You should probably use NewSerialResolver to create a new instance.
 //
-// Deprecated: please use ParallelResolver in new code.
+// Deprecated: please use ParallelResolver in new code. We cannot
+// remove this code as long as we use tracing for measuring.
+//
+// QUIRK: unlike the ParallelResolver, this resolver retries each
+// query three times for soft errors.
 type SerialResolver struct {
 	// Encoder is the MANDATORY encoder to use.
 	Encoder model.DNSEncoder
@@ -98,6 +102,8 @@ func (r *SerialResolver) LookupHTTPS(
 
 func (r *SerialResolver) lookupHostWithRetry(
 	ctx context.Context, hostname string, qtype uint16) ([]string, error) {
+	// QUIRK: retrying has been there since the beginning so we need to
+	// keep it as long as we're using tracing for measuring.
 	var errorslist []error
 	for i := 0; i < 3; i++ {
 		replies, err := r.lookupHostWithoutRetry(ctx, hostname, qtype)
@@ -116,9 +122,9 @@ func (r *SerialResolver) lookupHostWithRetry(
 		}
 		r.NumTimeouts.Add(1)
 	}
-	// bugfix: we MUST return one of the errors otherwise we confuse the
+	// QUIRK: we MUST return one of the errors otherwise we confuse the
 	// mechanism in errwrap that classifies the root cause operation, since
-	// it would not be able to find a child with a major operation error
+	// it would not be able to find a child with a major operation error.
 	return nil, errorslist[0]
 }
 
diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go
index f0a2fa5..7984587 100644
--- a/internal/netxlite/tls.go
+++ b/internal/netxlite/tls.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// TLS implementation
+//
+
 import (
 	"context"
 	"crypto/tls"
@@ -11,8 +15,12 @@ import (
 
 	oohttp "github.com/ooni/oohttp"
 	"github.com/ooni/probe-cli/v3/internal/model"
+	"github.com/ooni/probe-cli/v3/internal/runtimex"
 )
 
+// TODO(bassosimone): check whether there's now equivalent functionality
+// inside the standard library allowing us to map numbers to names.
+
 var (
 	tlsVersionString = map[uint16]string{
 		tls.VersionTLS10: "TLSv1",
@@ -81,7 +89,8 @@ func NewDefaultCertPool() *x509.CertPool {
 	pool := x509.NewCertPool()
 	// Assumption: AppendCertsFromPEM cannot fail because we
 	// have a test in certify_test.go that guarantees that
-	pool.AppendCertsFromPEM([]byte(pemcerts))
+	ok := pool.AppendCertsFromPEM([]byte(pemcerts))
+	runtimex.PanicIfFalse(ok, "pool.AppendCertsFromPEM failed")
 	return pool
 }
 
@@ -201,8 +210,8 @@ var defaultTLSHandshaker = &tlsHandshakerConfigurable{}
 
 // tlsHandshakerLogger is a TLSHandshaker with logging.
 type tlsHandshakerLogger struct {
-	model.TLSHandshaker
-	model.DebugLogger
+	TLSHandshaker model.TLSHandshaker
+	DebugLogger   model.DebugLogger
 }
 
 var _ model.TLSHandshaker = &tlsHandshakerLogger{}
@@ -313,7 +322,7 @@ func NewSingleUseTLSDialer(conn TLSConn) model.TLSDialer {
 // tlsDialerSingleUseAdapter adapts dialerSingleUse to
 // be a TLSDialer type rather than a Dialer type.
 type tlsDialerSingleUseAdapter struct {
-	model.Dialer
+	Dialer model.Dialer
 }
 
 var _ model.TLSDialer = &tlsDialerSingleUseAdapter{}
@@ -323,9 +332,13 @@ func (d *tlsDialerSingleUseAdapter) DialTLSContext(ctx context.Context, network,
 	return d.Dialer.DialContext(ctx, network, address)
 }
 
+func (d *tlsDialerSingleUseAdapter) CloseIdleConnections() {
+	d.Dialer.CloseIdleConnections()
+}
+
 // tlsHandshakerErrWrapper wraps the returned error to be an OONI error
 type tlsHandshakerErrWrapper struct {
-	model.TLSHandshaker
+	TLSHandshaker model.TLSHandshaker
 }
 
 // Handshake implements TLSHandshaker.Handshake
@@ -334,7 +347,7 @@ func (h *tlsHandshakerErrWrapper) Handshake(
 ) (net.Conn, tls.ConnectionState, error) {
 	tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config)
 	if err != nil {
-		return nil, tls.ConnectionState{}, NewErrWrapper(
+		return nil, tls.ConnectionState{}, newErrWrapper(
 			classifyTLSHandshakeError, TLSHandshakeOperation, err)
 	}
 	return tlsconn, state, nil
diff --git a/internal/netxlite/tls_test.go b/internal/netxlite/tls_test.go
index 2c17987..1fc8c47 100644
--- a/internal/netxlite/tls_test.go
+++ b/internal/netxlite/tls_test.go
@@ -487,6 +487,7 @@ func TestTLSDialer(t *testing.T) {
 func TestNewSingleUseTLSDialer(t *testing.T) {
 	conn := &mocks.TLSConn{}
 	d := NewSingleUseTLSDialer(conn)
+	defer d.CloseIdleConnections()
 	outconn, err := d.DialTLSContext(context.Background(), "", "")
 	if err != nil {
 		t.Fatal(err)
diff --git a/internal/netxlite/tproxy.go b/internal/netxlite/tproxy.go
index f5cbf1b..db366f1 100644
--- a/internal/netxlite/tproxy.go
+++ b/internal/netxlite/tproxy.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Transparent proxy (for integration testing)
+//
+
 import (
 	"context"
 	"net"
diff --git a/internal/netxlite/utls.go b/internal/netxlite/utls.go
index 5d668e2..a4ddc57 100644
--- a/internal/netxlite/utls.go
+++ b/internal/netxlite/utls.go
@@ -1,5 +1,9 @@
 package netxlite
 
+//
+// Code to use yawning/utls or refraction-networking/utls
+//
+
 import (
 	"context"
 	"crypto/tls"