urlgetter: expose the operation that failed (#680)

This is very useful to quickly understand what actually went wrong. It is
a facet of the next design that was never really used.

Also, acknowledge that, in some cases, there isn't a real failed operation
because the error happens before we enter into our network stack.

This happens, for example, when the context we're using is canceled.

To address this potential issue, introduce and use a `"top_level"` operation
that describes this specific situation.

This work is part the telegram rewrite: #646.

Also, with this diff landing we can close #311,
which mentioned oonimkall (the old implementation of netx) but is still
basically talking about the same concept we're implementing here.

experiment/urlgetter/getter.go

@@ -6,6 +6,8 @@ import (
 
 	"github.com/ooni/probe-engine/model"
 	"github.com/ooni/probe-engine/netx/archival"
+	"github.com/ooni/probe-engine/netx/errorx"
+	"github.com/ooni/probe-engine/netx/modelx"
 	"github.com/ooni/probe-engine/netx/trace"
 )
 
@@ -26,9 +28,14 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) {
 	begin := time.Now()
 	saver := new(trace.Saver)
 	tk, err := g.get(ctx, saver)
-	if err != nil {
-		tk.Failure = archival.NewFailure(err)
-	}
+	// Make sure we have an operation in cases where we fail before
+	// hitting our httptransport that does error wrapping.
+	err = errorx.SafeErrWrapperBuilder{
+		Error:     err,
+		Operation: modelx.TopLevelOperation,
+	}.MaybeBuild()
+	tk.FailedOperation = archival.NewFailedOperation(err)
+	tk.Failure = archival.NewFailure(err)
 	events := saver.Read()
 	tk.Queries = append(
 		tk.Queries, archival.NewDNSQueriesList(

experiment/urlgetter/getter_test.go

@@ -31,6 +31,9 @@ func TestGetterWithCancelledContextVanilla(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure == nil || !strings.HasSuffix(*tk.Failure, "context canceled") {
 		t.Fatal("not the Failure we expected")
 	}
@@ -90,6 +93,9 @@ func TestGetterWithCancelledContextAndMethod(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure == nil || !strings.HasSuffix(*tk.Failure, "context canceled") {
 		t.Fatal("not the Failure we expected")
 	}
@@ -151,6 +157,9 @@ func TestGetterWithCancelledContextNoFollowRedirects(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure == nil || !strings.HasSuffix(*tk.Failure, "context canceled") {
 		t.Fatal("not the Failure we expected")
 	}
@@ -211,7 +220,10 @@ func TestGetterWithCancelledContextCannotStartTunnel(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
-	if tk.Failure == nil || !strings.HasSuffix(*tk.Failure, "EOF") {
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
+	if tk.Failure == nil || *tk.Failure != "eof_error" {
 		t.Fatal("not the Failure we expected")
 	}
 	if len(tk.NetworkEvents) != 0 {
@@ -261,6 +273,9 @@ func TestGetterWithCancelledContextWithTunnel(t *testing.T) {
 	if tk.BootstrapTime != 10.0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure == nil || !strings.HasSuffix(*tk.Failure, "context canceled") {
 		t.Fatal("not the Failure we expected")
 	}
@@ -313,7 +328,7 @@ func TestGetterWithCancelledContextUnknownResolverURL(t *testing.T) {
 		Target:  "https://www.google.com",
 	}
 	tk, err := g.Get(ctx)
-	if err == nil || err.Error() != "unsupported resolver scheme" {
+	if err == nil || err.Error() != "unknown_failure: unsupported resolver scheme" {
 		t.Fatal("not the error we expected")
 	}
 	if tk.Agent != "redirect" {
@@ -322,6 +337,9 @@ func TestGetterWithCancelledContextUnknownResolverURL(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation == nil || *tk.FailedOperation != modelx.TopLevelOperation {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure == nil || *tk.Failure != "unknown_failure: unsupported resolver scheme" {
 		t.Fatal("not the Failure we expected")
 	}
@@ -367,6 +385,9 @@ func TestGetterIntegration(t *testing.T) {
 	if tk.BootstrapTime != 0 {
 		t.Fatal("not the BootstrapTime we expected")
 	}
+	if tk.FailedOperation != nil {
+		t.Fatal("not the FailedOperation we expected")
+	}
 	if tk.Failure != nil {
 		t.Fatal("not the Failure we expected")
 	}

experiment/urlgetter/urlgetter.go

@@ -31,17 +31,18 @@ type Config struct {
 
 // TestKeys contains the experiment's result.
 type TestKeys struct {
-	Agent         string                     `json:"agent"`
-	BootstrapTime float64                    `json:"bootstrap_time,omitempty"`
-	DNSCache      []string                   `json:"dns_cache,omitempty"`
-	Failure       *string                    `json:"failure"`
-	NetworkEvents []archival.NetworkEvent    `json:"network_events"`
-	Queries       []archival.DNSQueryEntry   `json:"queries"`
-	Requests      []archival.RequestEntry    `json:"requests"`
-	TCPConnect    []archival.TCPConnectEntry `json:"tcp_connect"`
-	SOCKSProxy    string                     `json:"socksproxy,omitempty"`
-	TLSHandshakes []archival.TLSHandshake    `json:"tls_handshakes"`
-	Tunnel        string                     `json:"tunnel,omitempty"`
+	Agent           string                     `json:"agent"`
+	BootstrapTime   float64                    `json:"bootstrap_time,omitempty"`
+	DNSCache        []string                   `json:"dns_cache,omitempty"`
+	FailedOperation *string                    `json:"failed_operation"`
+	Failure         *string                    `json:"failure"`
+	NetworkEvents   []archival.NetworkEvent    `json:"network_events"`
+	Queries         []archival.DNSQueryEntry   `json:"queries"`
+	Requests        []archival.RequestEntry    `json:"requests"`
+	TCPConnect      []archival.TCPConnectEntry `json:"tcp_connect"`
+	SOCKSProxy      string                     `json:"socksproxy,omitempty"`
+	TLSHandshakes   []archival.TLSHandshake    `json:"tls_handshakes"`
+	Tunnel          string                     `json:"tunnel,omitempty"`
 }
 
 // RegisterExtensions registers the extensions used by the urlgetter

netx/modelx/modelx.go

@@ -139,6 +139,10 @@ const (
 
 	// UnknownOperation is when we cannot determine the operation
 	UnknownOperation = "unknown"
+
+	// TopLevelOperation is used when the failure happens at top level. This
+	// happens for example with urlgetter with a cancelled context.
+	TopLevelOperation = "top_level"
 )
 
 // ErrWrapper is our error wrapper for Go errors. The key objective of