From 83440cf1108263ba6102a0a4b363b12e12e22ec3 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 7 Sep 2021 17:09:30 +0200 Subject: [PATCH] refactor: split errorsx in good and legacy (#477) The legacy part for now is internal/errorsx. It will stay there until I figure out whether it also needs some extra bug fixing. The good part is now in internal/netxlite/errorsx and contains all the logic for mapping errors. We need to further improve upon this logic by writing more thorough integration tests for QUIC. We also need to copy the various dialer, conn, etc adapters that set errors. We will put them inside netxlite and we will generate errors in a way that is less crazy with respect to the major operation. (The idea is to always wrap, given that now we measure in an incremental way and we don't measure every operation together.) Part of https://github.com/ooni/probe/issues/1591 --- .../internal/websteps/generate_test.go | 2 +- .../internal/websteps/measure_test.go | 2 +- internal/engine/experiment/dash/dash.go | 2 +- internal/engine/experiment/dash/dash_test.go | 2 +- .../experiment/fbmessenger/fbmessenger.go | 2 +- .../fbmessenger/fbmessenger_test.go | 2 +- internal/engine/experiment/hhfm/hhfm.go | 5 +- internal/engine/experiment/hhfm/hhfm_test.go | 2 +- internal/engine/experiment/hirl/hirl.go | 2 +- internal/engine/experiment/hirl/hirl_test.go | 2 +- .../experiment/riseupvpn/riseupvpn_test.go | 2 +- .../engine/experiment/signal/signal_test.go | 2 +- .../experiment/sniblocking/sniblocking.go | 2 +- .../sniblocking/sniblocking_test.go | 2 +- .../stunreachability/stunreachability_test.go | 2 +- .../engine/experiment/telegram/telegram.go | 2 +- .../experiment/telegram/telegram_test.go | 2 +- internal/engine/experiment/tor/tor.go | 2 +- internal/engine/experiment/tor/tor_test.go | 2 +- .../engine/experiment/urlgetter/getter.go | 5 +- .../urlgetter/getter_integration_test.go | 2 +- .../engine/experiment/urlgetter/runner.go | 2 +- .../experiment/webconnectivity/control.go | 5 +- .../experiment/webconnectivity/dnsanalysis.go | 2 +- .../webconnectivity/dnsanalysis_test.go | 2 +- .../experiment/webconnectivity/summary.go | 2 +- .../webconnectivity/summary_test.go | 2 +- .../webconnectivity/webconnectivity_test.go | 2 +- .../engine/experiment/websteps/control.go | 5 +- internal/engine/legacy/netx/http.go | 5 +- internal/engine/legacy/netx/http_test.go | 2 +- .../engine/legacy/netx/modelx/modelx_test.go | 2 +- .../netx/oldhttptransport/tracetripper.go | 9 +- .../legacy/oonidatamodel/oonidatamodel.go | 2 +- .../oonidatamodel/oonidatamodel_test.go | 2 +- .../oonitemplates/oonitemplates_test.go | 2 +- internal/engine/netx/archival/archival.go | 5 +- .../engine/netx/archival/archival_test.go | 2 +- internal/engine/netx/dialer/saver.go | 2 +- internal/engine/netx/dialer/saver_test.go | 2 +- internal/engine/netx/integration_test.go | 2 +- internal/engine/netx/quicdialer/system.go | 2 +- .../engine/netx/quicdialer/system_test.go | 2 +- internal/engine/netx/resolver/bogon.go | 2 +- internal/engine/netx/resolver/bogon_test.go | 2 +- internal/engine/netx/tlsdialer/saver_test.go | 2 +- internal/errorsx/dialer.go | 26 +- internal/errorsx/dialer_test.go | 25 +- internal/errorsx/errorsx.go | 135 +-------- internal/errorsx/errorsx_test.go | 242 +--------------- internal/errorsx/integration_test.go | 7 +- internal/errorsx/quic.go | 95 +------ internal/errorsx/quic_test.go | 7 +- internal/errorsx/resolver.go | 16 +- internal/errorsx/resolver_test.go | 7 +- internal/errorsx/tls.go | 30 +- internal/errorsx/tls_test.go | 7 +- internal/netxlite/errorsx/classify.go | 197 +++++++++++++ internal/netxlite/errorsx/classify_test.go | 260 ++++++++++++++++++ internal/netxlite/errorsx/doc.go | 2 + internal/{ => netxlite}/errorsx/errno.go | 2 +- internal/{ => netxlite}/errorsx/errno_test.go | 2 +- internal/{ => netxlite}/errorsx/errno_unix.go | 2 +- .../{ => netxlite}/errorsx/errno_windows.go | 2 +- internal/netxlite/errorsx/errwrapper.go | 51 ++++ internal/netxlite/errorsx/errwrapper_test.go | 24 ++ .../errorsx/internal/generrno/main.go | 0 internal/{ => netxlite}/errorsx/operations.go | 0 internal/netxlite/quirks.go | 2 +- internal/netxlite/quirks_test.go | 2 +- 70 files changed, 684 insertions(+), 576 deletions(-) create mode 100644 internal/netxlite/errorsx/classify.go create mode 100644 internal/netxlite/errorsx/classify_test.go create mode 100644 internal/netxlite/errorsx/doc.go rename internal/{ => netxlite}/errorsx/errno.go (98%) rename internal/{ => netxlite}/errorsx/errno_test.go (98%) rename internal/{ => netxlite}/errorsx/errno_unix.go (94%) rename internal/{ => netxlite}/errorsx/errno_windows.go (94%) create mode 100644 internal/netxlite/errorsx/errwrapper.go create mode 100644 internal/netxlite/errorsx/errwrapper_test.go rename internal/{ => netxlite}/errorsx/internal/generrno/main.go (100%) rename internal/{ => netxlite}/errorsx/operations.go (100%) diff --git a/internal/cmd/oohelperd/internal/websteps/generate_test.go b/internal/cmd/oohelperd/internal/websteps/generate_test.go index 0b0760e..93c8ab6 100644 --- a/internal/cmd/oohelperd/internal/websteps/generate_test.go +++ b/internal/cmd/oohelperd/internal/websteps/generate_test.go @@ -11,7 +11,7 @@ import ( "testing" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/runtimex" ) diff --git a/internal/cmd/oohelperd/internal/websteps/measure_test.go b/internal/cmd/oohelperd/internal/websteps/measure_test.go index 9c0dc5f..24730ee 100644 --- a/internal/cmd/oohelperd/internal/websteps/measure_test.go +++ b/internal/cmd/oohelperd/internal/websteps/measure_test.go @@ -6,7 +6,7 @@ import ( "net/url" "testing" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestMeasureSuccess(t *testing.T) { diff --git a/internal/engine/experiment/dash/dash.go b/internal/engine/experiment/dash/dash.go index 698dfbd..42077a9 100644 --- a/internal/engine/experiment/dash/dash.go +++ b/internal/engine/experiment/dash/dash.go @@ -17,8 +17,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/humanize" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" ) diff --git a/internal/engine/experiment/dash/dash_test.go b/internal/engine/experiment/dash/dash_test.go index 7376005..7151c2c 100644 --- a/internal/engine/experiment/dash/dash_test.go +++ b/internal/engine/experiment/dash/dash_test.go @@ -14,7 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestRunnerLoopLocateFailure(t *testing.T) { diff --git a/internal/engine/experiment/fbmessenger/fbmessenger.go b/internal/engine/experiment/fbmessenger/fbmessenger.go index ec13b96..75e6350 100644 --- a/internal/engine/experiment/fbmessenger/fbmessenger.go +++ b/internal/engine/experiment/fbmessenger/fbmessenger.go @@ -11,7 +11,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) const ( diff --git a/internal/engine/experiment/fbmessenger/fbmessenger_test.go b/internal/engine/experiment/fbmessenger/fbmessenger_test.go index 2fba4fe..33adb42 100644 --- a/internal/engine/experiment/fbmessenger/fbmessenger_test.go +++ b/internal/engine/experiment/fbmessenger/fbmessenger_test.go @@ -12,7 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/hhfm/hhfm.go b/internal/engine/experiment/hhfm/hhfm.go index c6fdffb..db9e4e9 100644 --- a/internal/engine/experiment/hhfm/hhfm.go +++ b/internal/engine/experiment/hhfm/hhfm.go @@ -19,7 +19,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" "github.com/ooni/probe-cli/v3/internal/randx" ) @@ -180,7 +181,7 @@ func Transact(txp Transport, req *http.Request, callbacks model.ExperimentCallbacks) (*http.Response, []byte, error) { // make sure that we return a wrapped error here resp, data, err := transact(txp, req, callbacks) - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: err, Operation: errorsx.TopLevelOperation}.MaybeBuild() return resp, data, err } diff --git a/internal/engine/experiment/hhfm/hhfm_test.go b/internal/engine/experiment/hhfm/hhfm_test.go index 24502db..8ca15e8 100644 --- a/internal/engine/experiment/hhfm/hhfm_test.go +++ b/internal/engine/experiment/hhfm/hhfm_test.go @@ -18,7 +18,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/hirl/hirl.go b/internal/engine/experiment/hirl/hirl.go index dc07730..b4b9e7f 100644 --- a/internal/engine/experiment/hirl/hirl.go +++ b/internal/engine/experiment/hirl/hirl.go @@ -14,7 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/randx" ) diff --git a/internal/engine/experiment/hirl/hirl_test.go b/internal/engine/experiment/hirl/hirl_test.go index e0bf183..1b13f6c 100644 --- a/internal/engine/experiment/hirl/hirl_test.go +++ b/internal/engine/experiment/hirl/hirl_test.go @@ -12,7 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/riseupvpn/riseupvpn_test.go b/internal/engine/experiment/riseupvpn/riseupvpn_test.go index e1672a7..ed61b54 100644 --- a/internal/engine/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/engine/experiment/riseupvpn/riseupvpn_test.go @@ -17,7 +17,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) const ( diff --git a/internal/engine/experiment/signal/signal_test.go b/internal/engine/experiment/signal/signal_test.go index ee2e19f..e3bff24 100644 --- a/internal/engine/experiment/signal/signal_test.go +++ b/internal/engine/experiment/signal/signal_test.go @@ -9,7 +9,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/sniblocking/sniblocking.go b/internal/engine/experiment/sniblocking/sniblocking.go index 4c4af64..d043da8 100644 --- a/internal/engine/experiment/sniblocking/sniblocking.go +++ b/internal/engine/experiment/sniblocking/sniblocking.go @@ -15,7 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) const ( diff --git a/internal/engine/experiment/sniblocking/sniblocking_test.go b/internal/engine/experiment/sniblocking/sniblocking_test.go index 078b7ed..fdb0081 100644 --- a/internal/engine/experiment/sniblocking/sniblocking_test.go +++ b/internal/engine/experiment/sniblocking/sniblocking_test.go @@ -9,7 +9,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) const ( diff --git a/internal/engine/experiment/stunreachability/stunreachability_test.go b/internal/engine/experiment/stunreachability/stunreachability_test.go index fe260cb..19e6077 100644 --- a/internal/engine/experiment/stunreachability/stunreachability_test.go +++ b/internal/engine/experiment/stunreachability/stunreachability_test.go @@ -12,7 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/stunreachability" "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/pion/stun" ) diff --git a/internal/engine/experiment/telegram/telegram.go b/internal/engine/experiment/telegram/telegram.go index f8a1cbf..2821591 100644 --- a/internal/engine/experiment/telegram/telegram.go +++ b/internal/engine/experiment/telegram/telegram.go @@ -11,7 +11,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) const ( diff --git a/internal/engine/experiment/telegram/telegram_test.go b/internal/engine/experiment/telegram/telegram_test.go index fd57dbc..9f8296b 100644 --- a/internal/engine/experiment/telegram/telegram_test.go +++ b/internal/engine/experiment/telegram/telegram_test.go @@ -12,7 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/tor/tor.go b/internal/engine/experiment/tor/tor.go index 436b7b7..c899b32 100644 --- a/internal/engine/experiment/tor/tor.go +++ b/internal/engine/experiment/tor/tor.go @@ -18,7 +18,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonidatamodel" "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/scrubber" ) diff --git a/internal/engine/experiment/tor/tor_test.go b/internal/engine/experiment/tor/tor_test.go index 89c3cff..1f3624c 100644 --- a/internal/engine/experiment/tor/tor_test.go +++ b/internal/engine/experiment/tor/tor_test.go @@ -17,7 +17,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" "github.com/ooni/probe-cli/v3/internal/engine/mockable" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/scrubber" ) diff --git a/internal/engine/experiment/urlgetter/getter.go b/internal/engine/experiment/urlgetter/getter.go index c85cdd0..bcaeb94 100644 --- a/internal/engine/experiment/urlgetter/getter.go +++ b/internal/engine/experiment/urlgetter/getter.go @@ -9,7 +9,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + legacyerrorsx "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/tunnel" ) @@ -54,7 +55,7 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) { tk, err := g.get(ctx, saver) // Make sure we have an operation in cases where we fail before // hitting our httptransport that does error wrapping. - err = errorsx.SafeErrWrapperBuilder{ + err = legacyerrorsx.SafeErrWrapperBuilder{ Error: err, Operation: errorsx.TopLevelOperation, }.MaybeBuild() diff --git a/internal/engine/experiment/urlgetter/getter_integration_test.go b/internal/engine/experiment/urlgetter/getter_integration_test.go index c1b5008..31e0b73 100644 --- a/internal/engine/experiment/urlgetter/getter_integration_test.go +++ b/internal/engine/experiment/urlgetter/getter_integration_test.go @@ -10,7 +10,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/mockable" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestGetterWithVeryShortTimeout(t *testing.T) { diff --git a/internal/engine/experiment/urlgetter/runner.go b/internal/engine/experiment/urlgetter/runner.go index 4f9f939..0d0f5e4 100644 --- a/internal/engine/experiment/urlgetter/runner.go +++ b/internal/engine/experiment/urlgetter/runner.go @@ -11,7 +11,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/httpheader" "github.com/ooni/probe-cli/v3/internal/engine/netx" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" "github.com/ooni/probe-cli/v3/internal/runtimex" ) diff --git a/internal/engine/experiment/webconnectivity/control.go b/internal/engine/experiment/webconnectivity/control.go index 29f0096..2abe148 100644 --- a/internal/engine/experiment/webconnectivity/control.go +++ b/internal/engine/experiment/webconnectivity/control.go @@ -6,7 +6,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/geolocate" "github.com/ooni/probe-cli/v3/internal/engine/httpx" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + legacyerrorsx "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // ControlRequest is the request that we send to the control @@ -59,7 +60,7 @@ func Control( } sess.Logger().Infof("control for %s...", creq.HTTPRequest) // make sure error is wrapped - err = errorsx.SafeErrWrapperBuilder{ + err = legacyerrorsx.SafeErrWrapperBuilder{ Error: clnt.PostJSON(ctx, "/", creq, &out), Operation: errorsx.TopLevelOperation, }.MaybeBuild() diff --git a/internal/engine/experiment/webconnectivity/dnsanalysis.go b/internal/engine/experiment/webconnectivity/dnsanalysis.go index cf103c3..9b8a916 100644 --- a/internal/engine/experiment/webconnectivity/dnsanalysis.go +++ b/internal/engine/experiment/webconnectivity/dnsanalysis.go @@ -4,7 +4,7 @@ import ( "net" "net/url" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // DNSAnalysisResult contains the results of analysing comparing diff --git a/internal/engine/experiment/webconnectivity/dnsanalysis_test.go b/internal/engine/experiment/webconnectivity/dnsanalysis_test.go index 47502fa..ad20eaf 100644 --- a/internal/engine/experiment/webconnectivity/dnsanalysis_test.go +++ b/internal/engine/experiment/webconnectivity/dnsanalysis_test.go @@ -7,7 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestDNSAnalysis(t *testing.T) { diff --git a/internal/engine/experiment/webconnectivity/summary.go b/internal/engine/experiment/webconnectivity/summary.go index 1416d5a..892fde4 100644 --- a/internal/engine/experiment/webconnectivity/summary.go +++ b/internal/engine/experiment/webconnectivity/summary.go @@ -5,7 +5,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity/internal" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // The following set of status flags identifies in a more nuanced way the diff --git a/internal/engine/experiment/webconnectivity/summary_test.go b/internal/engine/experiment/webconnectivity/summary_test.go index 2488d2b..b335f2d 100644 --- a/internal/engine/experiment/webconnectivity/summary_test.go +++ b/internal/engine/experiment/webconnectivity/summary_test.go @@ -7,7 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestSummarize(t *testing.T) { diff --git a/internal/engine/experiment/webconnectivity/webconnectivity_test.go b/internal/engine/experiment/webconnectivity/webconnectivity_test.go index 8dd2abb..4a5e79a 100644 --- a/internal/engine/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/engine/experiment/webconnectivity/webconnectivity_test.go @@ -13,7 +13,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewExperimentMeasurer(t *testing.T) { diff --git a/internal/engine/experiment/websteps/control.go b/internal/engine/experiment/websteps/control.go index da3fc9b..37fe0ab 100644 --- a/internal/engine/experiment/websteps/control.go +++ b/internal/engine/experiment/websteps/control.go @@ -5,7 +5,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/httpx" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // Control performs the control request and returns the response. @@ -18,7 +19,7 @@ func Control( Logger: sess.Logger(), } // make sure error is wrapped - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: clnt.PostJSON(ctx, resourcePath, creq, &out), Operation: errorsx.TopLevelOperation, }.MaybeBuild() diff --git a/internal/engine/legacy/netx/http.go b/internal/engine/legacy/netx/http.go index c62c483..01fa444 100644 --- a/internal/engine/legacy/netx/http.go +++ b/internal/engine/legacy/netx/http.go @@ -8,7 +8,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/handlers" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/oldhttptransport" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "golang.org/x/net/http2" ) @@ -82,7 +83,7 @@ func (t *HTTPTransport) RoundTrip( // For safety wrap the error as modelx.HTTPRoundTripOperation but this // will only be used if the error chain does not contain any // other major operation failure. See errorsx.ErrWrapper. - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: err, Operation: errorsx.HTTPRoundTripOperation, }.MaybeBuild() diff --git a/internal/engine/legacy/netx/http_test.go b/internal/engine/legacy/netx/http_test.go index 8de5267..5de2b57 100644 --- a/internal/engine/legacy/netx/http_test.go +++ b/internal/engine/legacy/netx/http_test.go @@ -13,7 +13,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" ) diff --git a/internal/engine/legacy/netx/modelx/modelx_test.go b/internal/engine/legacy/netx/modelx/modelx_test.go index 191bbc3..fd8ddd3 100644 --- a/internal/engine/legacy/netx/modelx/modelx_test.go +++ b/internal/engine/legacy/netx/modelx/modelx_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewTLSConnectionState(t *testing.T) { diff --git a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go index f074d9a..71e31a3 100644 --- a/internal/engine/legacy/netx/oldhttptransport/tracetripper.go +++ b/internal/engine/legacy/netx/oldhttptransport/tracetripper.go @@ -12,7 +12,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/atomicx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" ) @@ -116,7 +117,7 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) { TLSHandshakeDone: func(state tls.ConnectionState, err error) { // Wrapping the error even if we're not returning it because it may // less confusing to users to see the wrapped name - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: err, Operation: errorsx.TLSHandshakeOperation, }.MaybeBuild() @@ -171,7 +172,7 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) { WroteRequest: func(info httptrace.WroteRequestInfo) { // Wrapping the error even if we're not returning it because it may // less confusing to users to see the wrapped name - err := errorsx.SafeErrWrapperBuilder{ + err := errorsxlegacy.SafeErrWrapperBuilder{ Error: info.Err, Operation: errorsx.HTTPRoundTripOperation, }.MaybeBuild() @@ -207,7 +208,7 @@ func (t *TraceTripper) RoundTrip(req *http.Request) (*http.Response, error) { } resp, err := t.roundTripper.RoundTrip(req) - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: err, Operation: majorOp, }.MaybeBuild() diff --git a/internal/engine/legacy/oonidatamodel/oonidatamodel.go b/internal/engine/legacy/oonidatamodel/oonidatamodel.go index da6ac9b..f6d8229 100644 --- a/internal/engine/legacy/oonidatamodel/oonidatamodel.go +++ b/internal/engine/legacy/oonidatamodel/oonidatamodel.go @@ -19,8 +19,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // ExtSpec describes a data format extension diff --git a/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go b/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go index a6c0eae..95512cd 100644 --- a/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go +++ b/internal/engine/legacy/oonidatamodel/oonidatamodel_test.go @@ -12,7 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewTCPConnectListEmpty(t *testing.T) { diff --git a/internal/engine/legacy/oonitemplates/oonitemplates_test.go b/internal/engine/legacy/oonitemplates/oonitemplates_test.go index ba3287a..e06ed83 100644 --- a/internal/engine/legacy/oonitemplates/oonitemplates_test.go +++ b/internal/engine/legacy/oonitemplates/oonitemplates_test.go @@ -11,7 +11,7 @@ import ( goptlib "git.torproject.org/pluggable-transports/goptlib.git" "github.com/ooni/probe-cli/v3/internal/engine/legacy/netx/modelx" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "gitlab.com/yawning/obfs4.git/transports" obfs4base "gitlab.com/yawning/obfs4.git/transports/base" ) diff --git a/internal/engine/netx/archival/archival.go b/internal/engine/netx/archival/archival.go index 1d51f9b..2059442 100644 --- a/internal/engine/netx/archival/archival.go +++ b/internal/engine/netx/archival/archival.go @@ -19,7 +19,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/geolocate" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // ExtSpec describes a data format extension @@ -110,7 +111,7 @@ func NewFailure(err error) *string { // The following code guarantees that the error is always wrapped even // when we could not actually hit our code that does the wrapping. A case // in which this happen is with context deadline for HTTP. - err = errorsx.SafeErrWrapperBuilder{ + err = errorsxlegacy.SafeErrWrapperBuilder{ Error: err, Operation: errorsx.TopLevelOperation, }.MaybeBuild() diff --git a/internal/engine/netx/archival/archival_test.go b/internal/engine/netx/archival/archival_test.go index a9569ef..d8b8bc1 100644 --- a/internal/engine/netx/archival/archival_test.go +++ b/internal/engine/netx/archival/archival_test.go @@ -15,7 +15,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestNewTCPConnectList(t *testing.T) { diff --git a/internal/engine/netx/dialer/saver.go b/internal/engine/netx/dialer/saver.go index 75c9d25..cded652 100644 --- a/internal/engine/netx/dialer/saver.go +++ b/internal/engine/netx/dialer/saver.go @@ -6,7 +6,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // saverDialer saves events occurring during the dial diff --git a/internal/engine/netx/dialer/saver_test.go b/internal/engine/netx/dialer/saver_test.go index a09f419..3534023 100644 --- a/internal/engine/netx/dialer/saver_test.go +++ b/internal/engine/netx/dialer/saver_test.go @@ -9,7 +9,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) diff --git a/internal/engine/netx/integration_test.go b/internal/engine/netx/integration_test.go index 17ad622..3e2ede4 100644 --- a/internal/engine/netx/integration_test.go +++ b/internal/engine/netx/integration_test.go @@ -10,7 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/bytecounter" "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/iox" ) diff --git a/internal/engine/netx/quicdialer/system.go b/internal/engine/netx/quicdialer/system.go index a8b6e7b..57db87a 100644 --- a/internal/engine/netx/quicdialer/system.go +++ b/internal/engine/netx/quicdialer/system.go @@ -5,7 +5,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/quicx" ) diff --git a/internal/engine/netx/quicdialer/system_test.go b/internal/engine/netx/quicdialer/system_test.go index fe16fcd..6cc363a 100644 --- a/internal/engine/netx/quicdialer/system_test.go +++ b/internal/engine/netx/quicdialer/system_test.go @@ -10,8 +10,8 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite/quicx" ) diff --git a/internal/engine/netx/resolver/bogon.go b/internal/engine/netx/resolver/bogon.go index aab1667..63f40eb 100644 --- a/internal/engine/netx/resolver/bogon.go +++ b/internal/engine/netx/resolver/bogon.go @@ -4,7 +4,7 @@ import ( "context" "net" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/runtimex" ) diff --git a/internal/engine/netx/resolver/bogon_test.go b/internal/engine/netx/resolver/bogon_test.go index 0587009..d5c2d73 100644 --- a/internal/engine/netx/resolver/bogon_test.go +++ b/internal/engine/netx/resolver/bogon_test.go @@ -6,7 +6,7 @@ import ( "testing" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestResolverIsBogon(t *testing.T) { diff --git a/internal/engine/netx/tlsdialer/saver_test.go b/internal/engine/netx/tlsdialer/saver_test.go index 716e141..bc7449e 100644 --- a/internal/engine/netx/tlsdialer/saver_test.go +++ b/internal/engine/netx/tlsdialer/saver_test.go @@ -11,8 +11,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestSaverTLSHandshakerSuccessWithReadWrite(t *testing.T) { diff --git a/internal/errorsx/dialer.go b/internal/errorsx/dialer.go index 9e9a181..bca0c9a 100644 --- a/internal/errorsx/dialer.go +++ b/internal/errorsx/dialer.go @@ -3,6 +3,8 @@ package errorsx import ( "context" "net" + + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // Dialer establishes network connections. @@ -22,9 +24,9 @@ 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, &ErrWrapper{ - Failure: ClassifyGenericError(err), - Operation: ConnectOperation, + return nil, &errorsx.ErrWrapper{ + Failure: errorsx.ClassifyGenericError(err), + Operation: errorsx.ConnectOperation, WrappedErr: err, } } @@ -41,9 +43,9 @@ type errorWrapperConn struct { func (c *errorWrapperConn) Read(b []byte) (int, error) { count, err := c.Conn.Read(b) if err != nil { - return 0, &ErrWrapper{ - Failure: ClassifyGenericError(err), - Operation: ReadOperation, + return 0, &errorsx.ErrWrapper{ + Failure: errorsx.ClassifyGenericError(err), + Operation: errorsx.ReadOperation, WrappedErr: err, } } @@ -54,9 +56,9 @@ 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, &ErrWrapper{ - Failure: ClassifyGenericError(err), - Operation: WriteOperation, + return 0, &errorsx.ErrWrapper{ + Failure: errorsx.ClassifyGenericError(err), + Operation: errorsx.WriteOperation, WrappedErr: err, } } @@ -67,9 +69,9 @@ func (c *errorWrapperConn) Write(b []byte) (int, error) { func (c *errorWrapperConn) Close() error { err := c.Conn.Close() if err != nil { - return &ErrWrapper{ - Failure: ClassifyGenericError(err), - Operation: CloseOperation, + return &errorsx.ErrWrapper{ + Failure: errorsx.ClassifyGenericError(err), + Operation: errorsx.CloseOperation, WrappedErr: err, } } diff --git a/internal/errorsx/dialer_test.go b/internal/errorsx/dialer_test.go index ffd96e8..73eb532 100644 --- a/internal/errorsx/dialer_test.go +++ b/internal/errorsx/dialer_test.go @@ -7,6 +7,7 @@ import ( "net" "testing" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) @@ -18,14 +19,14 @@ func TestErrorWrapperDialerFailure(t *testing.T) { }, }} conn, err := d.DialContext(ctx, "tcp", "www.google.com:443") - var ew *ErrWrapper + var ew *errorsx.ErrWrapper if !errors.As(err, &ew) { t.Fatal("cannot convert to ErrWrapper") } - if ew.Operation != ConnectOperation { + if ew.Operation != errorsx.ConnectOperation { t.Fatal("unexpected operation", ew.Operation) } - if ew.Failure != FailureEOFError { + if ew.Failure != errorsx.FailureEOFError { t.Fatal("unexpected failure", ew.Failure) } if !errors.Is(ew.WrappedErr, io.EOF) { @@ -67,14 +68,14 @@ func TestErrorWrapperConnReadFailure(t *testing.T) { } buf := make([]byte, 1024) cnt, err := c.Read(buf) - var ew *ErrWrapper + var ew *errorsx.ErrWrapper if !errors.As(err, &ew) { t.Fatal("cannot cast error to ErrWrapper") } - if ew.Operation != ReadOperation { + if ew.Operation != errorsx.ReadOperation { t.Fatal("invalid operation", ew.Operation) } - if ew.Failure != FailureEOFError { + if ew.Failure != errorsx.FailureEOFError { t.Fatal("invalid failure", ew.Failure) } if !errors.Is(ew.WrappedErr, io.EOF) { @@ -113,14 +114,14 @@ func TestErrorWrapperConnWriteFailure(t *testing.T) { } buf := make([]byte, 1024) cnt, err := c.Write(buf) - var ew *ErrWrapper + var ew *errorsx.ErrWrapper if !errors.As(err, &ew) { t.Fatal("cannot cast error to ErrWrapper") } - if ew.Operation != WriteOperation { + if ew.Operation != errorsx.WriteOperation { t.Fatal("invalid operation", ew.Operation) } - if ew.Failure != FailureEOFError { + if ew.Failure != errorsx.FailureEOFError { t.Fatal("invalid failure", ew.Failure) } if !errors.Is(ew.WrappedErr, io.EOF) { @@ -158,14 +159,14 @@ func TestErrorWrapperConnCloseFailure(t *testing.T) { }, } err := c.Close() - var ew *ErrWrapper + var ew *errorsx.ErrWrapper if !errors.As(err, &ew) { t.Fatal("cannot cast error to ErrWrapper") } - if ew.Operation != CloseOperation { + if ew.Operation != errorsx.CloseOperation { t.Fatal("invalid operation", ew.Operation) } - if ew.Failure != FailureEOFError { + if ew.Failure != errorsx.FailureEOFError { t.Fatal("invalid failure", ew.Failure) } if !errors.Is(ew.WrappedErr, io.EOF) { diff --git a/internal/errorsx/errorsx.go b/internal/errorsx/errorsx.go index 2e34efe..757c3a8 100644 --- a/internal/errorsx/errorsx.go +++ b/internal/errorsx/errorsx.go @@ -2,69 +2,11 @@ package errorsx import ( - "context" "errors" - "fmt" - "strings" - "github.com/ooni/probe-cli/v3/internal/scrubber" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) -// ErrDNSBogon indicates that we found a bogon address. This is the -// correct value with which to initialize MeasurementRoot.ErrDNSBogon -// to tell this library to return an error when a bogon is found. -var ErrDNSBogon = errors.New("dns: detected bogon address") - -// 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. -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 - // error that we have not yet mapped to a failure. - Failure string - - // Operation is the operation that failed. If possible, it - // SHOULD be a _major_ operation. Major operations are: - // - // - ResolveOperation: resolving a domain name failed - // - ConnectOperation: connecting to an IP failed - // - TLSHandshakeOperation: TLS handshaking failed - // - HTTPRoundTripOperation: other errors during round trip - // - // Because a network connection doesn't necessarily know - // what is the current major operation we also have the - // following _minor_ operations: - // - // - CloseOperation: CLOSE failed - // - ReadOperation: READ failed - // - WriteOperation: WRITE failed - // - // If an ErrWrapper referring to a major operation is wrapping - // another ErrWrapper and such ErrWrapper already refers to - // a major operation, then the new ErrWrapper should use the - // child ErrWrapper major operation. Otherwise, it should use - // its own major operation. This way, the topmost wrapper is - // supposed to refer to the major operation that failed. - Operation string - - // WrappedErr is the error that we're wrapping. - WrappedErr error -} - -// Error returns a description of the error that occurred. -func (e *ErrWrapper) Error() string { - return e.Failure -} - -// Unwrap allows to access the underlying error -func (e *ErrWrapper) Unwrap() error { - return e.WrappedErr -} - // SafeErrWrapperBuilder contains a builder for ErrWrapper that // is safe, i.e., behaves correctly when the error is nil. type SafeErrWrapperBuilder struct { @@ -85,9 +27,9 @@ func (b SafeErrWrapperBuilder) MaybeBuild() (err error) { if b.Error != nil { classifier := b.Classifier if classifier == nil { - classifier = ClassifyGenericError + classifier = errorsx.ClassifyGenericError } - err = &ErrWrapper{ + err = &errorsx.ErrWrapper{ Failure: classifier(b.Error), Operation: toOperationString(b.Error, b.Operation), WrappedErr: b.Error, @@ -96,86 +38,31 @@ func (b SafeErrWrapperBuilder) MaybeBuild() (err error) { return } -// TODO (kelmenhorst, bassosimone): -// Use errors.Is / errors.As more often, when possible, in this classifier. -// These methods are more robust to library changes than strings. -// errors.Is / errors.As can only be used when the error is exported. - -// ClassifyGenericError is the generic classifier mapping an error -// occurred during an operation to an OONI failure string. -func ClassifyGenericError(err error) string { - // The list returned here matches the values used by MK unless - // explicitly noted otherwise with a comment. - - // TODO(bassosimone): we need to always apply this rule not only here - // when we're making the most generic conversion. - var errwrapper *ErrWrapper - if errors.As(err, &errwrapper) { - return errwrapper.Error() // we've already wrapped it - } - - if failure := classifySyscallError(err); failure != "" { - return failure - } - - if errors.Is(err, context.Canceled) { - return FailureInterrupted - } - s := err.Error() - if strings.HasSuffix(s, "operation was canceled") { - return FailureInterrupted - } - if strings.HasSuffix(s, "EOF") { - return FailureEOFError - } - if strings.HasSuffix(s, "context deadline exceeded") { - return FailureGenericTimeoutError - } - if strings.HasSuffix(s, "transaction is timed out") { - return FailureGenericTimeoutError - } - 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 - } - if strings.HasSuffix(s, "no such host") { - // This is dns_lookup_error in MK but such error is used as a - // generic "hey, the lookup failed" error. Instead, this error - // 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 -} - func toOperationString(err error, operation string) string { - var errwrapper *ErrWrapper + var errwrapper *errorsx.ErrWrapper if errors.As(err, &errwrapper) { // Basically, as explained in ErrWrapper docs, let's // keep the child major operation, if any. - if errwrapper.Operation == ConnectOperation { + if errwrapper.Operation == errorsx.ConnectOperation { return errwrapper.Operation } - if errwrapper.Operation == HTTPRoundTripOperation { + if errwrapper.Operation == errorsx.HTTPRoundTripOperation { return errwrapper.Operation } - if errwrapper.Operation == ResolveOperation { + if errwrapper.Operation == errorsx.ResolveOperation { return errwrapper.Operation } - if errwrapper.Operation == TLSHandshakeOperation { + if errwrapper.Operation == errorsx.TLSHandshakeOperation { return errwrapper.Operation } - if errwrapper.Operation == QUICHandshakeOperation { + if errwrapper.Operation == errorsx.QUICHandshakeOperation { return errwrapper.Operation } if errwrapper.Operation == "quic_handshake_start" { - return QUICHandshakeOperation + return errorsx.QUICHandshakeOperation } if errwrapper.Operation == "quic_handshake_done" { - return QUICHandshakeOperation + return errorsx.QUICHandshakeOperation } // FALLTHROUGH } diff --git a/internal/errorsx/errorsx_test.go b/internal/errorsx/errorsx_test.go index 32fd625..c5883ba 100644 --- a/internal/errorsx/errorsx_test.go +++ b/internal/errorsx/errorsx_test.go @@ -1,25 +1,17 @@ package errorsx import ( - "context" - "crypto/tls" - "crypto/x509" "errors" - "io" - "net" - "syscall" "testing" - "github.com/google/go-cmp/cmp" - "github.com/lucas-clemente/quic-go" - "github.com/pion/stun" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestMaybeBuildFactory(t *testing.T) { err := SafeErrWrapperBuilder{ Error: errors.New("mocked error"), }.MaybeBuild() - var target *ErrWrapper + var target *errorsx.ErrWrapper if errors.As(err, &target) == false { t.Fatal("not the expected error type") } @@ -31,242 +23,36 @@ func TestMaybeBuildFactory(t *testing.T) { } } -func TestToFailureString(t *testing.T) { - t.Run("for already wrapped error", func(t *testing.T) { - err := SafeErrWrapperBuilder{Error: io.EOF}.MaybeBuild() - 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() - <-ctx.Done() - if ClassifyGenericError(ctx.Err()) != FailureGenericTimeoutError { - 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 - conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", "www.google.com:80") - if err == nil { - t.Fatal("expected an error here") - } - if conn != nil { - t.Fatal("expected nil connection here") - } - if ClassifyGenericError(err) != FailureGenericTimeoutError { - 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", - }) != FailureDNSNXDOMAINError { - 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" - out := ClassifyGenericError(input) - if out != expected { - 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" - out := ClassifyGenericError(input) - if out != expected { - 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 - udpAddr := &net.UDPAddr{IP: net.ParseIP("216.58.212.164"), Port: 80, Zone: ""} - udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0}) - if err != nil { - t.Fatal(err) - } - sess, err := quic.DialEarlyContext(ctx, udpConn, udpAddr, "google.com:80", &tls.Config{}, &quic.Config{}) - if err == nil { - t.Fatal("expected an error here") - } - if sess != nil { - t.Fatal("expected nil session here") - } - if ClassifyGenericError(err) != FailureGenericTimeoutError { - t.Fatal("unexpected results") - } - }) -} - -func TestClassifyQUICFailure(t *testing.T) { - 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") - } - }) - -} - -func TestClassifyResolveFailure(t *testing.T) { - t.Run("for ErrDNSBogon", func(t *testing.T) { - if ClassifyResolverError(ErrDNSBogon) != FailureDNSBogonError { - t.Fatal("unexpected result") - } - }) -} - -func TestClassifyTLSFailure(t *testing.T) { - 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") - } - }) -} - func TestToOperationString(t *testing.T) { t.Run("for connect", func(t *testing.T) { // You're doing HTTP and connect fails. You want to know // that connect failed not that HTTP failed. - err := &ErrWrapper{Operation: ConnectOperation} - if toOperationString(err, HTTPRoundTripOperation) != ConnectOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.ConnectOperation} + if toOperationString(err, errorsx.HTTPRoundTripOperation) != errorsx.ConnectOperation { t.Fatal("unexpected result") } }) t.Run("for http_round_trip", func(t *testing.T) { // You're doing DoH and something fails inside HTTP. You want // to know about the internal HTTP error, not resolve. - err := &ErrWrapper{Operation: HTTPRoundTripOperation} - if toOperationString(err, ResolveOperation) != HTTPRoundTripOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.HTTPRoundTripOperation} + if toOperationString(err, errorsx.ResolveOperation) != errorsx.HTTPRoundTripOperation { t.Fatal("unexpected result") } }) t.Run("for resolve", func(t *testing.T) { // You're doing HTTP and the DNS fails. You want to // know that resolve failed. - err := &ErrWrapper{Operation: ResolveOperation} - if toOperationString(err, HTTPRoundTripOperation) != ResolveOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.ResolveOperation} + if toOperationString(err, errorsx.HTTPRoundTripOperation) != errorsx.ResolveOperation { t.Fatal("unexpected result") } }) t.Run("for tls_handshake", func(t *testing.T) { // You're doing HTTP and the TLS handshake fails. You want // to know about a TLS handshake error. - err := &ErrWrapper{Operation: TLSHandshakeOperation} - if toOperationString(err, HTTPRoundTripOperation) != TLSHandshakeOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.TLSHandshakeOperation} + if toOperationString(err, errorsx.HTTPRoundTripOperation) != errorsx.TLSHandshakeOperation { t.Fatal("unexpected result") } }) @@ -274,16 +60,16 @@ func TestToOperationString(t *testing.T) { // You just noticed that TLS handshake failed and you // have a child error telling you that read failed. Here // you want to know about a TLS handshake error. - err := &ErrWrapper{Operation: ReadOperation} - if toOperationString(err, TLSHandshakeOperation) != TLSHandshakeOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.ReadOperation} + if toOperationString(err, errorsx.TLSHandshakeOperation) != errorsx.TLSHandshakeOperation { t.Fatal("unexpected result") } }) t.Run("for quic_handshake", func(t *testing.T) { // You're doing HTTP and the TLS handshake fails. You want // to know about a TLS handshake error. - err := &ErrWrapper{Operation: QUICHandshakeOperation} - if toOperationString(err, HTTPRoundTripOperation) != QUICHandshakeOperation { + err := &errorsx.ErrWrapper{Operation: errorsx.QUICHandshakeOperation} + if toOperationString(err, errorsx.HTTPRoundTripOperation) != errorsx.QUICHandshakeOperation { t.Fatal("unexpected result") } }) diff --git a/internal/errorsx/integration_test.go b/internal/errorsx/integration_test.go index 187e0d2..832a975 100644 --- a/internal/errorsx/integration_test.go +++ b/internal/errorsx/integration_test.go @@ -6,8 +6,9 @@ import ( "testing" "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/errorsx" + errorsxlegacy "github.com/ooni/probe-cli/v3/internal/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestErrorWrapperQUICDialerInvalidCertificate(t *testing.T) { @@ -18,7 +19,7 @@ func TestErrorWrapperQUICDialerInvalidCertificate(t *testing.T) { ServerName: servername, } - dlr := &errorsx.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ + dlr := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }} // use Google IP @@ -41,7 +42,7 @@ func TestErrorWrapperQUICDialerSuccess(t *testing.T) { NextProtos: []string{"h3"}, ServerName: "www.google.com", } - d := &errorsx.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ + d := &errorsxlegacy.ErrorWrapperQUICDialer{Dialer: &netxlite.QUICDialerQUICGo{ QUICListener: &netxlite.QUICListenerStdlib{}, }} sess, err := d.DialContext(ctx, "udp", "216.58.212.164:443", tlsConf, &quic.Config{}) diff --git a/internal/errorsx/quic.go b/internal/errorsx/quic.go index 818fb02..f94149f 100644 --- a/internal/errorsx/quic.go +++ b/internal/errorsx/quic.go @@ -3,10 +3,10 @@ package errorsx import ( "context" "crypto/tls" - "errors" "net" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/quicx" ) @@ -39,7 +39,7 @@ func (qls *ErrorWrapperQUICListener) Listen(addr *net.UDPAddr) (quicx.UDPLikeCon if err != nil { return nil, SafeErrWrapperBuilder{ Error: err, - Operation: QUICListenOperation, + Operation: errorsx.QUICListenOperation, }.MaybeBuild() } return &errorWrapperUDPConn{pconn}, nil @@ -59,7 +59,7 @@ func (c *errorWrapperUDPConn) WriteTo(p []byte, addr net.Addr) (int, error) { if err != nil { return 0, SafeErrWrapperBuilder{ Error: err, - Operation: WriteToOperation, + Operation: errorsx.WriteToOperation, }.MaybeBuild() } return count, nil @@ -71,7 +71,7 @@ func (c *errorWrapperUDPConn) ReadFrom(b []byte) (int, net.Addr, error) { if err != nil { return 0, nil, SafeErrWrapperBuilder{ Error: err, - Operation: ReadFromOperation, + Operation: errorsx.ReadFromOperation, }.MaybeBuild() } return n, addr, nil @@ -88,95 +88,12 @@ func (d *ErrorWrapperQUICDialer) DialContext( tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { sess, err := d.Dialer.DialContext(ctx, network, host, tlsCfg, cfg) err = SafeErrWrapperBuilder{ - Classifier: ClassifyQUICHandshakeError, + Classifier: errorsx.ClassifyQUICHandshakeError, Error: err, - Operation: QUICHandshakeOperation, + Operation: errorsx.QUICHandshakeOperation, }.MaybeBuild() if err != nil { return nil, err } return sess, nil } - -// ClassifyQUICHandshakeError maps an error occurred during the QUIC -// handshake to an OONI failure string. -func ClassifyQUICHandshakeError(err error) string { - var versionNegotiation *quic.VersionNegotiationError - var statelessReset *quic.StatelessResetError - var handshakeTimeout *quic.HandshakeTimeoutError - var idleTimeout *quic.IdleTimeoutError - var transportError *quic.TransportError - - if errors.As(err, &versionNegotiation) { - return FailureQUICIncompatibleVersion - } - if errors.As(err, &statelessReset) { - return FailureConnectionReset - } - if errors.As(err, &handshakeTimeout) { - return FailureGenericTimeoutError - } - if errors.As(err, &idleTimeout) { - return FailureGenericTimeoutError - } - if errors.As(err, &transportError) { - if transportError.ErrorCode == quic.ConnectionRefused { - return FailureConnectionRefused - } - // the TLS Alert constants are taken from RFC8446 - errCode := uint8(transportError.ErrorCode) - if quicIsCertificateError(errCode) { - return FailureSSLInvalidCertificate - } - // TLSAlertDecryptError and TLSAlertHandshakeFailure are summarized to a 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 - } - if errCode == quicTLSAlertUnknownCA { - return FailureSSLUnknownAuthority - } - if errCode == quicTLSUnrecognizedName { - return FailureSSLInvalidHostname - } - } - return ClassifyGenericError(err) -} - -// TLS alert protocol as defined in RFC8446 -const ( - // Sender was unable to negotiate an acceptable set of security parameters given the options available. - quicTLSAlertHandshakeFailure = 40 - - // Certificate was corrupt, contained signatures that did not verify correctly, etc. - quicTLSAlertBadCertificate = 42 - - // Certificate was of an unsupported type. - quicTLSAlertUnsupportedCertificate = 43 - - // Certificate was revoked by its signer. - quicTLSAlertCertificateRevoked = 44 - - // Certificate has expired or is not currently valid. - quicTLSAlertCertificateExpired = 45 - - // Some unspecified issue arose in processing the certificate, rendering it unacceptable. - quicTLSAlertCertificateUnknown = 46 - - // Certificate was not accepted because the CA certificate could not be located or could not be matched with a known trust anchor. - quicTLSAlertUnknownCA = 48 - - // Handshake (not record layer) cryptographic operation failed. - quicTLSAlertDecryptError = 51 - - // Sent by servers when no server exists identified by the name provided by the client via the "server_name" extension. - quicTLSUnrecognizedName = 112 -) - -func quicIsCertificateError(alert uint8) bool { - return (alert == quicTLSAlertBadCertificate || - alert == quicTLSAlertUnsupportedCertificate || - alert == quicTLSAlertCertificateExpired || - alert == quicTLSAlertCertificateRevoked || - alert == quicTLSAlertCertificateUnknown) -} diff --git a/internal/errorsx/quic_test.go b/internal/errorsx/quic_test.go index ca3c21c..90fa05c 100644 --- a/internal/errorsx/quic_test.go +++ b/internal/errorsx/quic_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite/quicx" ) @@ -142,14 +143,14 @@ func TestErrorWrapperQUICDialerFailure(t *testing.T) { if !errors.Is(err, io.EOF) { t.Fatal("expected another error here") } - var errWrapper *ErrWrapper + var errWrapper *errorsx.ErrWrapper if !errors.As(err, &errWrapper) { t.Fatal("cannot cast to ErrWrapper") } - if errWrapper.Operation != QUICHandshakeOperation { + if errWrapper.Operation != errorsx.QUICHandshakeOperation { t.Fatal("unexpected Operation") } - if errWrapper.Failure != FailureEOFError { + if errWrapper.Failure != errorsx.FailureEOFError { t.Fatal("unexpected failure") } } diff --git a/internal/errorsx/resolver.go b/internal/errorsx/resolver.go index 42c960c..2b5a01d 100644 --- a/internal/errorsx/resolver.go +++ b/internal/errorsx/resolver.go @@ -2,7 +2,8 @@ package errorsx import ( "context" - "errors" + + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // Resolver is a DNS resolver. The *net.Resolver used by Go implements @@ -23,22 +24,13 @@ var _ Resolver = &ErrorWrapperResolver{} func (r *ErrorWrapperResolver) LookupHost(ctx context.Context, hostname string) ([]string, error) { addrs, err := r.Resolver.LookupHost(ctx, hostname) err = SafeErrWrapperBuilder{ - Classifier: ClassifyResolverError, + Classifier: errorsx.ClassifyResolverError, Error: err, - Operation: ResolveOperation, + Operation: errorsx.ResolveOperation, }.MaybeBuild() return addrs, err } -// ClassifyResolverError maps an error occurred during a domain name -// resolution to the corresponding OONI failure string. -func ClassifyResolverError(err error) string { - if errors.Is(err, ErrDNSBogon) { - return FailureDNSBogonError // not in MK - } - return ClassifyGenericError(err) -} - type resolverNetworker interface { Network() string } diff --git a/internal/errorsx/resolver_test.go b/internal/errorsx/resolver_test.go index c4f9f78..df41fd1 100644 --- a/internal/errorsx/resolver_test.go +++ b/internal/errorsx/resolver_test.go @@ -6,6 +6,7 @@ import ( "net" "testing" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) @@ -40,14 +41,14 @@ func TestErrorWrapperResolverFailure(t *testing.T) { if addrs != nil { t.Fatal("expected nil addr here") } - var errWrapper *ErrWrapper + var errWrapper *errorsx.ErrWrapper if !errors.As(err, &errWrapper) { t.Fatal("cannot properly cast the returned error") } - if errWrapper.Failure != FailureDNSNXDOMAINError { + if errWrapper.Failure != errorsx.FailureDNSNXDOMAINError { t.Fatal("unexpected failure") } - if errWrapper.Operation != ResolveOperation { + if errWrapper.Operation != errorsx.ResolveOperation { t.Fatal("unexpected Operation") } } diff --git a/internal/errorsx/tls.go b/internal/errorsx/tls.go index 2c05d51..2011562 100644 --- a/internal/errorsx/tls.go +++ b/internal/errorsx/tls.go @@ -3,9 +3,9 @@ package errorsx import ( "context" "crypto/tls" - "crypto/x509" - "errors" "net" + + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // TLSHandshaker is the generic TLS handshaker @@ -25,31 +25,9 @@ func (h *ErrorWrapperTLSHandshaker) Handshake( ) (net.Conn, tls.ConnectionState, error) { tlsconn, state, err := h.TLSHandshaker.Handshake(ctx, conn, config) err = SafeErrWrapperBuilder{ - Classifier: ClassifyTLSHandshakeError, + Classifier: errorsx.ClassifyTLSHandshakeError, Error: err, - Operation: TLSHandshakeOperation, + Operation: errorsx.TLSHandshakeOperation, }.MaybeBuild() return tlsconn, state, err } - -// ClassifyTLSHandshakeError maps an error occurred during the TLS -// handshake to an OONI failure string. -func ClassifyTLSHandshakeError(err error) string { - var x509HostnameError x509.HostnameError - if errors.As(err, &x509HostnameError) { - // Test case: https://wrong.host.badssl.com/ - return FailureSSLInvalidHostname - } - var x509UnknownAuthorityError x509.UnknownAuthorityError - if errors.As(err, &x509UnknownAuthorityError) { - // Test case: https://self-signed.badssl.com/. This error has - // never been among the ones returned by MK. - return FailureSSLUnknownAuthority - } - var x509CertificateInvalidError x509.CertificateInvalidError - if errors.As(err, &x509CertificateInvalidError) { - // Test case: https://expired.badssl.com/ - return FailureSSLInvalidCertificate - } - return ClassifyGenericError(err) -} diff --git a/internal/errorsx/tls_test.go b/internal/errorsx/tls_test.go index 8773a70..954fc98 100644 --- a/internal/errorsx/tls_test.go +++ b/internal/errorsx/tls_test.go @@ -8,6 +8,7 @@ import ( "net" "testing" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) @@ -29,14 +30,14 @@ func TestErrorWrapperTLSHandshakerFailure(t *testing.T) { if conn != nil { t.Fatal("expected nil con here") } - var errWrapper *ErrWrapper + var errWrapper *errorsx.ErrWrapper if !errors.As(err, &errWrapper) { t.Fatal("cannot cast to ErrWrapper") } - if errWrapper.Failure != FailureEOFError { + if errWrapper.Failure != errorsx.FailureEOFError { t.Fatal("unexpected Failure") } - if errWrapper.Operation != TLSHandshakeOperation { + if errWrapper.Operation != errorsx.TLSHandshakeOperation { t.Fatal("unexpected Operation") } } diff --git a/internal/netxlite/errorsx/classify.go b/internal/netxlite/errorsx/classify.go new file mode 100644 index 0000000..3c2a6f3 --- /dev/null +++ b/internal/netxlite/errorsx/classify.go @@ -0,0 +1,197 @@ +package errorsx + +import ( + "context" + "crypto/x509" + "errors" + "fmt" + "strings" + + "github.com/lucas-clemente/quic-go" + "github.com/ooni/probe-cli/v3/internal/scrubber" +) + +// TODO (kelmenhorst, bassosimone): +// Use errors.Is / errors.As more often, when possible, in this classifier. +// These methods are more robust to library changes than strings. +// errors.Is / errors.As can only be used when the error is exported. + +// ClassifyGenericError is the generic classifier mapping an error +// occurred during an operation to an OONI failure string. +func ClassifyGenericError(err error) string { + // The list returned here matches the values used by MK unless + // explicitly noted otherwise with a comment. + + var errwrapper *ErrWrapper + if errors.As(err, &errwrapper) { + return errwrapper.Error() // we've already wrapped it + } + + if failure := classifySyscallError(err); failure != "" { + return failure + } + + if errors.Is(err, context.Canceled) { + return FailureInterrupted + } + s := err.Error() + if strings.HasSuffix(s, "operation was canceled") { + return FailureInterrupted + } + if strings.HasSuffix(s, "EOF") { + return FailureEOFError + } + if strings.HasSuffix(s, "context deadline exceeded") { + return FailureGenericTimeoutError + } + if strings.HasSuffix(s, "transaction is timed out") { + return FailureGenericTimeoutError + } + 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 + } + if strings.HasSuffix(s, "no such host") { + // This is dns_lookup_error in MK but such error is used as a + // generic "hey, the lookup failed" error. Instead, this error + // 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 +const ( + // Sender was unable to negotiate an acceptable set of security parameters given the options available. + quicTLSAlertHandshakeFailure = 40 + + // Certificate was corrupt, contained signatures that did not verify correctly, etc. + quicTLSAlertBadCertificate = 42 + + // Certificate was of an unsupported type. + quicTLSAlertUnsupportedCertificate = 43 + + // Certificate was revoked by its signer. + quicTLSAlertCertificateRevoked = 44 + + // Certificate has expired or is not currently valid. + quicTLSAlertCertificateExpired = 45 + + // Some unspecified issue arose in processing the certificate, rendering it unacceptable. + quicTLSAlertCertificateUnknown = 46 + + // Certificate was not accepted because the CA certificate could not be located or could not be matched with a known trust anchor. + quicTLSAlertUnknownCA = 48 + + // Handshake (not record layer) cryptographic operation failed. + quicTLSAlertDecryptError = 51 + + // Sent by servers when no server exists identified by the name provided by the client via the "server_name" extension. + quicTLSUnrecognizedName = 112 +) + +func quicIsCertificateError(alert uint8) bool { + return (alert == quicTLSAlertBadCertificate || + alert == quicTLSAlertUnsupportedCertificate || + alert == quicTLSAlertCertificateExpired || + alert == quicTLSAlertCertificateRevoked || + alert == quicTLSAlertCertificateUnknown) +} + +// ClassifyQUICHandshakeError maps an error occurred during the QUIC +// handshake to an OONI failure string. +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 statelessReset *quic.StatelessResetError + var handshakeTimeout *quic.HandshakeTimeoutError + var idleTimeout *quic.IdleTimeoutError + var transportError *quic.TransportError + + if errors.As(err, &versionNegotiation) { + return FailureQUICIncompatibleVersion + } + if errors.As(err, &statelessReset) { + return FailureConnectionReset + } + if errors.As(err, &handshakeTimeout) { + return FailureGenericTimeoutError + } + if errors.As(err, &idleTimeout) { + return FailureGenericTimeoutError + } + if errors.As(err, &transportError) { + if transportError.ErrorCode == quic.ConnectionRefused { + return FailureConnectionRefused + } + // the TLS Alert constants are taken from RFC8446 + errCode := uint8(transportError.ErrorCode) + if quicIsCertificateError(errCode) { + return FailureSSLInvalidCertificate + } + // TLSAlertDecryptError and TLSAlertHandshakeFailure are summarized to a 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 + } + if errCode == quicTLSAlertUnknownCA { + return FailureSSLUnknownAuthority + } + if errCode == quicTLSUnrecognizedName { + return FailureSSLInvalidHostname + } + } + return ClassifyGenericError(err) +} + +// ErrDNSBogon indicates that we found a bogon address. This is the +// correct value with which to initialize MeasurementRoot.ErrDNSBogon +// 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. +func ClassifyResolverError(err error) string { + var errwrapper *ErrWrapper + if errors.As(err, &errwrapper) { + return errwrapper.Error() // we've already wrapped it + } + if errors.Is(err, ErrDNSBogon) { + return FailureDNSBogonError // not in MK + } + return ClassifyGenericError(err) +} + +// ClassifyTLSHandshakeError maps an error occurred during the TLS +// handshake to an OONI failure string. +func ClassifyTLSHandshakeError(err error) string { + var errwrapper *ErrWrapper + if errors.As(err, &errwrapper) { + return errwrapper.Error() // we've already wrapped it + } + var x509HostnameError x509.HostnameError + if errors.As(err, &x509HostnameError) { + // Test case: https://wrong.host.badssl.com/ + return FailureSSLInvalidHostname + } + var x509UnknownAuthorityError x509.UnknownAuthorityError + if errors.As(err, &x509UnknownAuthorityError) { + // Test case: https://self-signed.badssl.com/. This error has + // never been among the ones returned by MK. + return FailureSSLUnknownAuthority + } + var x509CertificateInvalidError x509.CertificateInvalidError + if errors.As(err, &x509CertificateInvalidError) { + // Test case: https://expired.badssl.com/ + return FailureSSLInvalidCertificate + } + return ClassifyGenericError(err) +} diff --git a/internal/netxlite/errorsx/classify_test.go b/internal/netxlite/errorsx/classify_test.go new file mode 100644 index 0000000..5a5bf7d --- /dev/null +++ b/internal/netxlite/errorsx/classify_test.go @@ -0,0 +1,260 @@ +package errorsx + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "io" + "net" + "syscall" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/lucas-clemente/quic-go" + "github.com/pion/stun" +) + +func TestClassifyGenericError(t *testing.T) { + t.Run("for input being already an ErrWrapper", func(t *testing.T) { + err := &ErrWrapper{Failure: FailureEOFError} + if ClassifyGenericError(err) != FailureEOFError { + 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() + <-ctx.Done() + if ClassifyGenericError(ctx.Err()) != FailureGenericTimeoutError { + 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 + conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", "www.google.com:80") + if err == nil { + t.Fatal("expected an error here") + } + if conn != nil { + t.Fatal("expected nil connection here") + } + if ClassifyGenericError(err) != FailureGenericTimeoutError { + 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", + }) != FailureDNSNXDOMAINError { + 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" + out := ClassifyGenericError(input) + if out != expected { + 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" + out := ClassifyGenericError(input) + if out != expected { + 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 + udpAddr := &net.UDPAddr{IP: net.ParseIP("216.58.212.164"), Port: 80, Zone: ""} + udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4zero, Port: 0}) + if err != nil { + t.Fatal(err) + } + sess, err := quic.DialEarlyContext(ctx, udpConn, udpAddr, "google.com:80", &tls.Config{}, &quic.Config{}) + if err == nil { + t.Fatal("expected an error here") + } + if sess != nil { + t.Fatal("expected nil session here") + } + if ClassifyGenericError(err) != FailureGenericTimeoutError { + t.Fatal("unexpected results") + } + }) +} + +func TestClassifyQUICHandshakeError(t *testing.T) { + t.Run("for input being already an ErrWrapper", func(t *testing.T) { + err := &ErrWrapper{Failure: FailureEOFError} + if ClassifyQUICHandshakeError(err) != FailureEOFError { + 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") + } + }) +} + +func TestClassifyResolverError(t *testing.T) { + t.Run("for input being already an ErrWrapper", func(t *testing.T) { + err := &ErrWrapper{Failure: FailureEOFError} + if ClassifyResolverError(err) != FailureEOFError { + 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") + } + }) +} + +func TestClassifyTLSHandshakeError(t *testing.T) { + t.Run("for input being already an ErrWrapper", func(t *testing.T) { + err := &ErrWrapper{Failure: FailureEOFError} + if ClassifyTLSHandshakeError(err) != FailureEOFError { + 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") + } + }) +} diff --git a/internal/netxlite/errorsx/doc.go b/internal/netxlite/errorsx/doc.go new file mode 100644 index 0000000..6070b6e --- /dev/null +++ b/internal/netxlite/errorsx/doc.go @@ -0,0 +1,2 @@ +// Package errorsx contains code to classify errors. +package errorsx diff --git a/internal/errorsx/errno.go b/internal/netxlite/errorsx/errno.go similarity index 98% rename from internal/errorsx/errno.go rename to internal/netxlite/errorsx/errno.go index fb873d4..16fc5bd 100644 --- a/internal/errorsx/errno.go +++ b/internal/netxlite/errorsx/errno.go @@ -1,5 +1,5 @@ // Code generated by go generate; DO NOT EDIT. -// Generated: 2021-09-07 15:15:03.350386 +0200 CEST m=+0.135456751 +// Generated: 2021-09-07 16:43:08.462721 +0200 CEST m=+0.105415376 package errorsx diff --git a/internal/errorsx/errno_test.go b/internal/netxlite/errorsx/errno_test.go similarity index 98% rename from internal/errorsx/errno_test.go rename to internal/netxlite/errorsx/errno_test.go index cee89b5..5264c66 100644 --- a/internal/errorsx/errno_test.go +++ b/internal/netxlite/errorsx/errno_test.go @@ -1,5 +1,5 @@ // Code generated by go generate; DO NOT EDIT. -// Generated: 2021-09-07 15:15:03.398087 +0200 CEST m=+0.183158793 +// Generated: 2021-09-07 16:43:08.510432 +0200 CEST m=+0.153127376 package errorsx diff --git a/internal/errorsx/errno_unix.go b/internal/netxlite/errorsx/errno_unix.go similarity index 94% rename from internal/errorsx/errno_unix.go rename to internal/netxlite/errorsx/errno_unix.go index 05fbc9a..b03294c 100644 --- a/internal/errorsx/errno_unix.go +++ b/internal/netxlite/errorsx/errno_unix.go @@ -1,5 +1,5 @@ // Code generated by go generate; DO NOT EDIT. -// Generated: 2021-09-07 15:15:03.215384 +0200 CEST m=+0.000452543 +// Generated: 2021-09-07 16:43:08.35751 +0200 CEST m=+0.000202959 package errorsx diff --git a/internal/errorsx/errno_windows.go b/internal/netxlite/errorsx/errno_windows.go similarity index 94% rename from internal/errorsx/errno_windows.go rename to internal/netxlite/errorsx/errno_windows.go index b3f8b5b..b9ec61d 100644 --- a/internal/errorsx/errno_windows.go +++ b/internal/netxlite/errorsx/errno_windows.go @@ -1,5 +1,5 @@ // Code generated by go generate; DO NOT EDIT. -// Generated: 2021-09-07 15:15:03.324258 +0200 CEST m=+0.109328501 +// Generated: 2021-09-07 16:43:08.436584 +0200 CEST m=+0.079277834 package errorsx diff --git a/internal/netxlite/errorsx/errwrapper.go b/internal/netxlite/errorsx/errwrapper.go new file mode 100644 index 0000000..e5b31a7 --- /dev/null +++ b/internal/netxlite/errorsx/errwrapper.go @@ -0,0 +1,51 @@ +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. +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 + // error that we have not yet mapped to a failure. + Failure string + + // Operation is the operation that failed. If possible, it + // SHOULD be a _major_ operation. Major operations are: + // + // - ResolveOperation: resolving a domain name failed + // - ConnectOperation: connecting to an IP failed + // - TLSHandshakeOperation: TLS handshaking failed + // - HTTPRoundTripOperation: other errors during round trip + // + // Because a network connection doesn't necessarily know + // what is the current major operation we also have the + // following _minor_ operations: + // + // - CloseOperation: CLOSE failed + // - ReadOperation: READ failed + // - WriteOperation: WRITE failed + // + // If an ErrWrapper referring to a major operation is wrapping + // another ErrWrapper and such ErrWrapper already refers to + // a major operation, then the new ErrWrapper should use the + // child ErrWrapper major operation. Otherwise, it should use + // its own major operation. This way, the topmost wrapper is + // supposed to refer to the major operation that failed. + Operation string + + // WrappedErr is the error that we're wrapping. + WrappedErr error +} + +// Error returns a description of the error that occurred. +func (e *ErrWrapper) Error() string { + return e.Failure +} + +// Unwrap allows to access the underlying error +func (e *ErrWrapper) Unwrap() error { + return e.WrappedErr +} diff --git a/internal/netxlite/errorsx/errwrapper_test.go b/internal/netxlite/errorsx/errwrapper_test.go new file mode 100644 index 0000000..021cc31 --- /dev/null +++ b/internal/netxlite/errorsx/errwrapper_test.go @@ -0,0 +1,24 @@ +package errorsx + +import ( + "errors" + "io" + "testing" +) + +func TestErrWrapperError(t *testing.T) { + err := &ErrWrapper{Failure: FailureDNSNXDOMAINError} + if err.Error() != FailureDNSNXDOMAINError { + t.Fatal("invalid return value") + } +} + +func TestErrWrapperUnwrap(t *testing.T) { + err := &ErrWrapper{ + Failure: FailureEOFError, + WrappedErr: io.EOF, + } + if !errors.Is(err, io.EOF) { + t.Fatal("cannot unwrap error") + } +} diff --git a/internal/errorsx/internal/generrno/main.go b/internal/netxlite/errorsx/internal/generrno/main.go similarity index 100% rename from internal/errorsx/internal/generrno/main.go rename to internal/netxlite/errorsx/internal/generrno/main.go diff --git a/internal/errorsx/operations.go b/internal/netxlite/errorsx/operations.go similarity index 100% rename from internal/errorsx/operations.go rename to internal/netxlite/errorsx/operations.go diff --git a/internal/netxlite/quirks.go b/internal/netxlite/quirks.go index dc3af6b..a70ee2a 100644 --- a/internal/netxlite/quirks.go +++ b/internal/netxlite/quirks.go @@ -4,7 +4,7 @@ import ( "errors" "strings" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) // This file contains weird stuff that we carried over from diff --git a/internal/netxlite/quirks_test.go b/internal/netxlite/quirks_test.go index 2488332..47c88fe 100644 --- a/internal/netxlite/quirks_test.go +++ b/internal/netxlite/quirks_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/errorsx" + "github.com/ooni/probe-cli/v3/internal/netxlite/errorsx" ) func TestQuirkReduceErrors(t *testing.T) {