Skip to content

Commit

Permalink
Handle go mod wrapped errors
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
illicitonion committed Apr 26, 2022
1 parent b5c1a9f commit 56d35f8
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 17 deletions.
53 changes: 42 additions & 11 deletions language/go/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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}
}
Expand Down
106 changes: 100 additions & 6 deletions language/go/update_import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
{
Expand Down Expand Up @@ -228,7 +229,7 @@ go_repository(
wantErr: "",
stubGoModDownload: nil,
}, {
desc: "modules-with-error",
desc: "modules-download-error",
files: []testtools.FileSpec{
{
Path: "go.mod",
Expand All @@ -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")
},
}, {
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 56d35f8

Please sign in to comment.