That seems reasonable to me, but I'd like to confirm with @FiloSottile or @rsc .

(CC @jayconrod)

There are always going to be ways the go command fails and prints to standard error without producing JSON on standard output. Why not treat this failure like any other such failure? Why does it have to be encoded in JSON on standard output?

Another option is maybe base.Fatalf should be printing the JSON during -json mode. That would be a much smaller patch.

There are always going to be ways the go command fails and prints to standard error without producing JSON on standard output

Does that mean that go proxies should always try to parse json from stdout (on failure and success) and if the parsing fails they need to read stderr as the alternate failure message?

Why not treat this failure like any other such failure? Why does it have to be encoded in JSON on standard output?

That's kind of what I expected. "Any other such failure" does produce valid json, therefore I expected the sumdb failure to also behave similarly.

For example the following command fails, but produces valid json:

go mod download -json github.com/asdfasdf/asdfasdfasdf@v1.1.0
go: finding github.com/asdfasdf/asdfasdfasdf v1.1.0
go: finding github.com/asdfasdf/asdfasdfasdf v1.1.0
{
	"Path": "github.com/asdfasdf/asdfasdfasdf",
	"Version": "v1.1.0",
	"Error": "github.com/asdfasdf/asdfasdfasdf@v1.1.0: invalid version: unknown revision v1.1.0"
}

While the same exact command fails (for a different reason), but does not produce a valid json:

go mod download -json github.com/marwan-at-work/umlaut@latest
go: finding github.com/marwan-at-work/umlaut latest
verifying github.com/marwan-at-work/umlaut@v0.0.0-20180801060001-9b04c86ca3e2/go.mod: github.com/marwan-at-work/umlaut@v0.0.0-20180801060001-9b04c86ca3e2/go.mod: reading https://sum.golang.org/lookup/github.com/marwan-at-work/umlaut@v0.0.0-20180801060001-9b04c86ca3e2: 410 Gone

umlaut is a test private repo in my github account.

Another option is maybe base.Fatalf should be printing the JSON during -json mode. That would be a much smaller patch.

The issue is that some json data will be lost or may not even be the same shape, for example: go mod download -json returns a json of this shape https://github.com/golang/go/blob/master/src/cmd/go/internal/modcmd/download.go#L59:L70

While go list can return either of these:
https://github.com/golang/go/blob/master///src/cmd/go/internal/load/pkg.go#L51:6
https://github.com/golang/go/blob/master///src/cmd/go/internal/modinfo/info.go#L12:6

So I imagine base.Fatalf will probably have its own json shape that describes the error and potential extra information.

That sounds good but the issue is that at this moment, running go mod download -json and running go list -m may "fail" through a base.Fatalf but may also fail through a propagated return err. Which means go proxies will have to continue guessing as to what the json shape will be.

If the patch is to change all errors to use base.Fatalf shape, then that will be consistent, but I imagine that's an equally large change to the codebase.

But maybe you had something else in mind for how the go proxies should deal with go mod download -json and go list -m failures?

Thanks for the quick response!

I agree that proxies should be robust to missing JSON output.

On the other hand, I suspect that we will need to convert many calls to base.Fatalf to return structured errors anyway, both so that they can be diagnosed more easily (the point of failure doesn't always have enough contextual information, such as the import or requirement path by which the failed module was reached), and so that explicitly error-tolerant tools such as gopls can degrade gracefully when working on incomplete or inconsistent code.

So I'm not sure that go mod download -json in particular is a good motivating example, but we may need to fix it incidentally anyway.

cc @hyangah, @katiehockman

Change https://golang.org/cl/197062 mentions this issue: cmd/go: consistent output for -json failures