Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: define error codes and use them to describe errors. #30134

Open
hyangah opened this issue Feb 8, 2019 · 3 comments
Open

cmd/go: define error codes and use them to describe errors. #30134

hyangah opened this issue Feb 8, 2019 · 3 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@hyangah
Copy link
Contributor

hyangah commented Feb 8, 2019

Go command, particularly, go list and go mod download, now serves as the canonical way to retrieve information about modules and packages. Programs and libraries that need to understand how Go handles build and interprets source code are supposed to invoke the command and interpret the command's output. For example, golang.org/x/tools/go/packages depends on invocation of go list, and we expect some module proxies to utilize go mod download or go list.

Go command provides -json and other flags to output in a structured form to ease the result parsing. But, handling error cases programmatically is still difficult. To be useful like a library, Go command line tool output should allow programs to distinguish different failure cases (invalid arguments, failed precondition, resource unavailability (network, tool, disk, ...) , permission issue, ...).

Currently,

  • Command line exit code: Go command exits with a non-zero exit code for "some" error cases (often 1). But this exit code doesn't carry enough information. Depending on the exit code or error messages from a command line anyway is not a reliable way.

  • Error/Err fields, as in -json or go list -e -f results, provide some error details, but they are "string" types. Parsing and depending on the message is not reliable or scalable.

$ go mod download -json golang.org/x/foo@v1.0.1
{
	"Path": "golang.org/x/foo",
	"Version": "v1.0.1",
	"Error": "unrecognized import path \"golang.org/x/foo\" (parse https://golang.org/x/foo?go-get=1: no go-import meta tags ())"
}
$ go mod download -json golang.org/x/text@v0.3.7
go: finding golang.org/x/text v0.3.7
{
	"Path": "golang.org/x/text",
	"Version": "v0.3.7",
	"Error": "unknown revision v0.3.7"
}

One option is that we define a set of status codes (like https://github.com/grpc/grpc/blob/master/doc/statuscodes.md or like HTTP, JSON error codes)
and use that to describe the kind of the error.

(sidenote: I wished Go2 error proposals also covered encoding/decoding of the error types and root causes but it seems that was not discussed.)

Moreover, we also need to fix the Go command to report the root cause of the error accurately.
For example, I ran the following example while I had not network access. Even though the root cause of the failure is the (temporary) network issue, the error message is not distinguishable from what Go returns when the module does not exists.

$ go list -m --versions --json -e golang.org/x/text
{
        "Path": "golang.org/x/text",
        "Error": {
                "Err": "module \"golang.org/x/text\" is not a known dependency"
        }
}

I am happy to file a separate issue about misleading error messages if there is no existing open issue yet.

@jayconrod @bcmills
@katiehockman @heschik @ianthehat

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 8, 2019
@andybons andybons added this to the Go1.13 milestone Feb 8, 2019
@jayconrod jayconrod added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Feb 11, 2019
@jayconrod jayconrod self-assigned this Feb 11, 2019
@rsc
Copy link
Contributor

rsc commented Feb 14, 2019

In general the go command does not know an accurate error. It ran a helper program like git and that command fails. Trying to pull accurate error analysis out of those helper programs is basically futile.

If the go command return an error code, what would a caller do differently? The right behavior may be just to retry in 1 minute (or 2 or 3 or 5 or whatever, maybe even with exponential backoff) regardless of what went wrong.

I was involved with the great error code page discussions that led to the gRPC HTTP error codes, and honestly I am skeptical that they are that useful in practice. I know the go command does not have available the information necessary to select the right one.

Even "permanent" errors are only permanent until a person fixes them.

@marwan-at-work
Copy link
Contributor

marwan-at-work commented Mar 11, 2019

@rsc error codes from go would be highly beneficial and important for a GOPROXY for two reasons:

  1. Be able to differentiate whether an error is expected or unexpected. This way, when a team runs a GOPROXY at scale, we want to be alerted if go mod download is returning unexpected error codes indicating something is wrong and worth looking into. Or, for example, if a client requests a non-existent version, then that error is not worth alerting for since it's a bad client request.
    Such expected errors that can be ignored or acted upon are: RepositoryNotFound, VersionNotFound, AuthenticationFailed, IncorrectModulePath (if it does not begin with host name) and potentially more.
    Right now the only option is to manually monitor error logs because we'd either never be alerted on anything or always be alerted on everything. This is in my opinion very important to figure out.

  2. The GOPROXY might need specific behavior based on the error codes. For example, the Athens proxy would like to know that if we run go list -m -versions, it could return a RepositoryNotFound error. Or it can also return an "unexpected" error from VCS. This way we can determine consistent and reliable behavior on whether we serve the cache results or fail/warn the user that our answer is a partial answer for the /list endpoint.

There are also other examples about determining behavior based on error codes which I'm happy to mention as well if need be.

@rsc
Copy link
Contributor

rsc commented Mar 14, 2019

I think we should leave this for Go 1.14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants