From b7786a7324658b9b1a7e496c9b883be35215fc2d Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 7 Sep 2021 21:18:26 +0200 Subject: [PATCH] refactor(netxlite/errorsx): extract string-suffix classifier (#482) This change makes the code more tidy and easier to read. No functional change, though. See https://github.com/ooni/probe/issues/1591. --- internal/netxlite/errorsx/classify.go | 21 ++++++++++++------- internal/netxlite/errorsx/errno.go | 5 +---- internal/netxlite/errorsx/errno_test.go | 2 +- internal/netxlite/errorsx/errno_unix.go | 2 +- internal/netxlite/errorsx/errno_windows.go | 2 +- .../errorsx/internal/generrno/main.go | 3 --- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/internal/netxlite/errorsx/classify.go b/internal/netxlite/errorsx/classify.go index 35941a0..a6b463f 100644 --- a/internal/netxlite/errorsx/classify.go +++ b/internal/netxlite/errorsx/classify.go @@ -11,11 +11,6 @@ import ( "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. // @@ -58,6 +53,18 @@ func ClassifyGenericError(err error) string { return FailureInterrupted } + if failure := classifyWithStringSuffix(err); failure != "" { + return failure + } + + formatted := fmt.Sprintf("unknown_failure: %s", err.Error()) + return scrubber.Scrub(formatted) // scrub IP addresses in the error +} + +// classifyWithStringSuffix is a subset of ClassifyGenericError that +// performs classification by looking at error suffixes. This function +// will return an empty string if it cannot classify the error. +func classifyWithStringSuffix(err error) string { s := err.Error() if strings.HasSuffix(s, "operation was canceled") { return FailureInterrupted @@ -83,9 +90,7 @@ func ClassifyGenericError(err error) string { // that we return here is significantly more specific. return FailureDNSNXDOMAINError } - - formatted := fmt.Sprintf("unknown_failure: %s", s) - return scrubber.Scrub(formatted) // scrub IP addresses in the error + return "" // not found } // TLS alert protocol as defined in RFC8446. We need these definitions diff --git a/internal/netxlite/errorsx/errno.go b/internal/netxlite/errorsx/errno.go index 892147a..9bdb1d0 100644 --- a/internal/netxlite/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 20:26:06.502417 +0200 CEST m=+0.133209876 +// Generated: 2021-09-07 20:42:20.172357 +0200 CEST m=+0.135433792 package errorsx @@ -64,9 +64,6 @@ const ( // proper OONI error. Returns the OONI error string // on success, an empty string otherwise. func classifySyscallError(err error) string { - // filter out system errors: necessary to detect all windows errors - // https://github.com/ooni/probe/issues/1526 describes the problem - // of mapping localized windows errors. var errno syscall.Errno if !errors.As(err, &errno) { return "" diff --git a/internal/netxlite/errorsx/errno_test.go b/internal/netxlite/errorsx/errno_test.go index 8599d2f..9e14921 100644 --- a/internal/netxlite/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 20:26:06.547786 +0200 CEST m=+0.178579584 +// Generated: 2021-09-07 20:42:20.217324 +0200 CEST m=+0.180405501 package errorsx diff --git a/internal/netxlite/errorsx/errno_unix.go b/internal/netxlite/errorsx/errno_unix.go index ba8e36c..8bd90a2 100644 --- a/internal/netxlite/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 20:26:06.370246 +0200 CEST m=+0.001036417 +// Generated: 2021-09-07 20:42:20.037485 +0200 CEST m=+0.000558667 package errorsx diff --git a/internal/netxlite/errorsx/errno_windows.go b/internal/netxlite/errorsx/errno_windows.go index 750e10e..713ef75 100644 --- a/internal/netxlite/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 20:26:06.478577 +0200 CEST m=+0.109369751 +// Generated: 2021-09-07 20:42:20.147482 +0200 CEST m=+0.110557751 package errorsx diff --git a/internal/netxlite/errorsx/internal/generrno/main.go b/internal/netxlite/errorsx/internal/generrno/main.go index 4b12e4f..578f0ea 100644 --- a/internal/netxlite/errorsx/internal/generrno/main.go +++ b/internal/netxlite/errorsx/internal/generrno/main.go @@ -198,9 +198,6 @@ func writeGenericFile() { fileWrite(filep, "// proper OONI error. Returns the OONI error string\n") fileWrite(filep, "// on success, an empty string otherwise.\n") fileWrite(filep, "func classifySyscallError(err error) string {\n") - fileWrite(filep, "\t// filter out system errors: necessary to detect all windows errors\n") - fileWrite(filep, "\t// https://github.com/ooni/probe/issues/1526 describes the problem\n") - fileWrite(filep, "\t// of mapping localized windows errors.\n") fileWrite(filep, "\tvar errno syscall.Errno\n") fileWrite(filep, "\tif !errors.As(err, &errno) {\n") fileWrite(filep, "\t\treturn \"\"\n")