Handle wrapped errors

[illicitonion]

What type of PR is this?

Bug fix

What package or component does this PR mostly affect?

language/go

What does this PR do? Why is it needed?

Handle 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.

[achew22]

Forgive my naivete on this one, but it seems that you're creating a new struct, which you don't create any instance of, and then call a method on it? Are you depending on struct equivalence to call your unwrap method?

Can we get away with doing a type switch on this to decide if it is an error or string and then put a struct in that has both of those as a fields and returns the one that is non-zero in the Unwrap method?

[illicitonion]

We're using Go's built-in json deserialization here: so while we're not manually creating an instance of the jsonError struct, Go is doing so for us. This StackOverflow post suggested that this is the cleanest way to have the json deserializer automatically deserialize either a string or struct.

I don't think here's a handy hook-point where we can insert our own logic here, as we're relying on the stdlib json.Unmarshal, but will gladly restructure if you can nudge me at how :)

[sluongng]

I would suggest adding a test for this change, including an assertion for how go mod download -json outputs would look like if InvalidPathError was to return and how that is failing the decode. Then provide a fix and flip the test assertion.

That would help maintainers review and merge this change a lot easier (without having to go and read up all the link reference).

It's weird how you are experiencing decoding issue since we are essentially following go documentation https://golang.org/ref/mod#go-mod-download here:

> go mod help download
...
The -json flag causes download to print a sequence of JSON objects
to standard output, describing each downloaded module (or failure),
corresponding to this Go struct:

    type Module struct {
        Path     string // module path
        Version  string // module version
        Error    string // error loading module
        Info     string // absolute path to cached .info file
        GoMod    string // absolute path to cached .mod file
        Zip      string // absolute path to cached .zip file
        Dir      string // absolute path to cached source root directory
        Sum      string // checksum for path, version (as in go.sum)
        GoModSum string // checksum for go.mod (as in go.sum)
    }
...

> go version
go version go1.18.1 darwin/arm64

I am still not yet sure on the fix direction just yet, but I can review again once the test is provided.

[illicitonion]

Sorry for the incomplete PR - I did some digging, and found that the root cause is actually that we use the same code to parse go list and go mod download output, but they actually use different formats. Added some tests for both cases - PTAL!

[sluongng]

Ah so the output of go mod download and go list are being decoded into type module struct.

And because go list recently updated so we have these differences

// go list
type Module struct {
    ...
    Error      *ModuleError // error loading module
}

// go mod download
type Module struct {
    ...
    Error    string // error loading module
}

hence the proposal is to split module struct into 2 different structs: moduleFromDownload and moduleFromList to avoid decode error. This fix direction LGTM.

[illicitonion]

All addressed - PTAL

[sluongng]

latest version LGTM

@achew22 could you take a look?

[achew22]

This is much cleaner than that was happening before. Thank you for fixing it!