Handle go mod wrapped errors

`go mod download` returns a different error structure than `go list`, as documented at https://go.dev/ref/mod#go-mod-download and https://go.dev/ref/mod#go-list-m We were previously parsing these assuming they had the same structure, instead, parse them separately. Also, prevent infinite looping on invalid JSON.
bazel-contrib · Apr 26, 2022 · 56d35f8 · 56d35f8
1 parent b5c1a9f
commit 56d35f8
Show file tree

Hide file tree

Showing 2 changed files with 142 additions and 17 deletions.
diff --git a/language/go/modules.go b/language/go/modules.go
@@ -40,22 +40,37 @@ func importReposFromModules(args language.ImportReposArgs) language.ImportReposR
 
 	// List all modules except for the main module, including implicit indirect
 	// dependencies.
-	type module struct {
+	// Schema is documented at https://go.dev/ref/mod#go-list-m
+	type moduleFromList struct {
 		Path, Version, Sum, Error string
 		Main                      bool
 		Replace                   *struct {
 			Path, Version string
 		}
 	}
 	// path@version can be used as a unique identifier for looking up sums
-	pathToModule := map[string]*module{}
+	pathToModule := map[string]*moduleFromList{}
 	data, err := goListModules(dir)
+	dec := json.NewDecoder(bytes.NewReader(data))
 	if err != nil {
+		// Best-effort try to adorn specific error details from the JSON output.
+		for dec.More() {
+			var dl moduleFromList
+			if decodeErr := dec.Decode(&dl); decodeErr != nil {
+				// If we couldn't parse a possible error description, just return the raw error.
+				err = fmt.Errorf("%w\nError parsing module for more error information: %v", err, decodeErr)
+				break
+			}
+			if dl.Error != "" {
+				err = fmt.Errorf("%w\nError listing %v: %v", err, dl.Path, dl.Error)
+			}
+		}
+		err = fmt.Errorf("error from go list: %w", err)
+
 		return language.ImportReposResult{Error: err}
 	}
-	dec := json.NewDecoder(bytes.NewReader(data))
 	for dec.More() {
-		mod := new(module)
+		mod := new(moduleFromList)
 		if err := dec.Decode(mod); err != nil {
 			return language.ImportReposResult{Error: err}
 		}
@@ -103,6 +118,20 @@ func importReposFromModules(args language.ImportReposArgs) language.ImportReposR
 		}
 	}
 
+	type downloadError struct {
+		Err string
+	}
+
+	// Schema is documented at https://go.dev/ref/mod#go-mod-download
+	type moduleFromDownload struct {
+		Path, Version, Sum string
+		Main               bool
+		Replace            *struct {
+			Path, Version string
+		}
+		Error *downloadError
+	}
+
 	if len(missingSumArgs) > 0 {
 		tmpDir, err := ioutil.TempDir("", "")
 		if err != nil {
@@ -114,20 +143,22 @@ func importReposFromModules(args language.ImportReposArgs) language.ImportReposR
 		if err != nil {
 			// Best-effort try to adorn specific error details from the JSON output.
 			for dec.More() {
-				var dl module
-				if err := dec.Decode(&dl); err != nil {
-					// If we couldn't parse a possible error description, just ignore this part of the output.
-					continue
+				var dl moduleFromDownload
+				if decodeErr := dec.Decode(&dl); decodeErr != nil {
+					// If we couldn't parse a possible error description, just return the raw error.
+					err = fmt.Errorf("%w\nError parsing module for more error information: %v", err, decodeErr)
+					break
 				}
-				if dl.Error != "" {
-					err = fmt.Errorf("%v\nError downloading %v: %v", err, dl.Path, dl.Error)
+				if dl.Error != nil {
+					err = fmt.Errorf("%w\nError downloading %v: %v", err, dl.Path, dl.Error.Err)
 				}
 			}
+			err = fmt.Errorf("error from go mod download: %w", err)
 
 			return language.ImportReposResult{Error: err}
 		}
 		for dec.More() {
-			var dl module
+			var dl moduleFromDownload
 			if err := dec.Decode(&dl); err != nil {
 				return language.ImportReposResult{Error: err}
 			}

diff --git a/language/go/update_import_test.go b/language/go/update_import_test.go
@@ -33,6 +33,7 @@ func TestImports(t *testing.T) {
 		desc, want        string
 		wantErr           string
 		stubGoModDownload func(string, []string) ([]byte, error)
+		stubGoListModules func(string) ([]byte, error)
 		files             []testtools.FileSpec
 	}{
 		{
@@ -228,7 +229,7 @@ go_repository(
 			wantErr:           "",
 			stubGoModDownload: nil,
 		}, {
-			desc: "modules-with-error",
+			desc: "modules-download-error",
 			files: []testtools.FileSpec{
 				{
 					Path: "go.mod",
@@ -242,17 +243,105 @@ require (
 				}, {
 					Path: "go.sum",
 					Content: `
-					definitely.doesnotexist/ever v0.1.0/go.mod h1:HI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuJ=
-					`,
+definitely.doesnotexist/ever v0.1.0/go.mod h1:HI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuJ=
+`,
+				},
+			},
+			want:    "",
+			wantErr: "error from go mod download: failed to download\nError downloading definitely.doesnotexist/ever: Did not exist",
+			stubGoModDownload: func(dir string, args []string) ([]byte, error) {
+				return []byte(`{
+"Path": "definitely.doesnotexist/ever",
+"Version": "0.1.0",
+"Error": {
+    "Err": "Did not exist"
+}
+}`), fmt.Errorf("failed to download")
+			},
+		}, {
+			desc: "modules-download-bad-json",
+			files: []testtools.FileSpec{
+				{
+					Path: "go.mod",
+					Content: `
+module github.com/bazelbuild/bazel-gazelle
+
+require (
+	definitely.doesnotexist/ever v0.1.0
+)
+`,
+				}, {
+					Path: "go.sum",
+					Content: `
+definitely.doesnotexist/ever v0.1.0/go.mod h1:HI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuJ=
+`,
 				},
 			},
 			want:    "",
-			wantErr: "failed to download\nError downloading definitely.doesnotexist/ever: Did not exist",
+			wantErr: "error from go mod download: failed to download\nError parsing module for more error information: invalid character 'o' in literal null (expecting 'u')",
 			stubGoModDownload: func(dir string, args []string) ([]byte, error) {
 				return []byte(`{
-	"Path": "definitely.doesnotexist/ever",
+"Path": "definitely.doesnotexist/ever",
+"Version": "0.1.0",
+"Error": {
+   "Err": not valid json
+}
+}`), fmt.Errorf("failed to download")
+			},
+		}, {
+			desc: "list-modules-error",
+			files: []testtools.FileSpec{
+				{
+					Path: "go.mod",
+					Content: `
+module github.com/bazelbuild/bazel-gazelle
+
+require (
+	definitely.doesnotexist/ever v0.1.0
+)
+`,
+				}, {
+					Path: "go.sum",
+					Content: `
+definitely.doesnotexist/ever v0.1.0/go.mod h1:HI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuJ=
+`,
+				},
+			},
+			want:    "",
+			wantErr: "error from go list: failed to download\nError listing definitely.doesnotexist/ever: Did not exist",
+			stubGoListModules: func(dir string) ([]byte, error) {
+				return []byte(`{
+"Path": "definitely.doesnotexist/ever",
+"Version": "0.1.0",
+"Error": "Did not exist"
+}`), fmt.Errorf("failed to download")
+			},
+		}, {
+			desc: "list-modules-bad-json",
+			files: []testtools.FileSpec{
+				{
+					Path: "go.mod",
+					Content: `
+module github.com/bazelbuild/bazel-gazelle
+
+require (
+	definitely.doesnotexist/ever v0.1.0
+)
+`,
+				}, {
+					Path: "go.sum",
+					Content: `
+definitely.doesnotexist/ever v0.1.0/go.mod h1:HI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuJ=
+`,
+				},
+			},
+			want:    "",
+			wantErr: "error from go list: failed to download\nError parsing module for more error information: invalid character 'n' after object key",
+			stubGoListModules: func(dir string) ([]byte, error) {
+				return []byte(`{
+    "Path": "definitely.doesnotexist/ever",
     "Version": "0.1.0",
-    "Error": "Did not exist"
+    "Error" not valid json
 }`), fmt.Errorf("failed to download")
 			},
 		}, {
@@ -349,6 +438,11 @@ go_repository(
 				goModDownload = tc.stubGoModDownload
 				defer func() { goModDownload = previousGoModDownload }()
 			}
+			if tc.stubGoListModules != nil {
+				previousGoListModules := goListModules
+				goListModules = tc.stubGoListModules
+				defer func() { goListModules = previousGoListModules }()
+			}
 			dir, cleanup := testtools.CreateFiles(t, tc.files)
 			defer cleanup()