Possibility to distinguish ErrMissingDriver

[se7entyse7en]

AFAIU the only errors exposed by the client are these. Is it possible to also distinguish ErrMissingDriver error? In our opinion handling this error differently is common as can be seen here.

[creachadair]

It seems like a generally useful thing to have. I'm a little surprised we aren't propagating some kind of standard machine-readable codes here (whether the set from gRPC or otherwise).

[se7entyse7en]

This is currently an example of missing driver, and the gRPC error code is Unknown.

✘  se7entyse7en in ~/Projects/.go-workspace/src/github.com/src-d/engine (master)  $ go run ./cmd/srcd/main.go parse uast .travis.yml                                             [01/04/19 | 18:38:01.045]
Error: could not stream: rpc error: code = Unknown desc = could not parse: rpc error: code = Unknown desc = unexpected error: runtime failure: missing driver for language "yaml"```

[bzz]

@se7entyse7en true, here is the relevant place where this gets exposed protocol/service.go#L127 and here is how the client works this around bblfshctl/cmd/driver_install.go#L343

@creachadair do you have any insights on what's the best way to propagate these though gRPC? gRPC metadata or something else?

[creachadair]

@se7entyse7en true, here is the relevant place where this gets exposed protocol/service.go#L127 and here is how the client works this around bblfshctl/cmd/driver_install.go#L343

@creachadair do you have any insights on what's the best way to propagate these though gRPC? gRPC metadata or something else?

For errors, I would recommend we try to use https://godoc.org/google.golang.org/grpc/status to construct error responses, and set appropriate codes. The *Status type has a code that we can use (probably codes.NotFound in this case), and if necessary we can attach extra details that will be propagated in the response.

On the client side, the client can use status.FromError to retrieve the *Status value, and inspect the code, message, and/or details.

The exact API varies by language—what I described above uses the Go libraries—but the status mechanism is common to gRPC.

[bzz]

@se7entyse7en true, here is the relevant place where this gets exposed protocol/service.go#L127 and here is how the client works this around bblfshctl/cmd/driver_install.go#L343
@creachadair do you have any insights on what's the best way to propagate these though gRPC? gRPC metadata or something else?

For errors, I would recommend we try to use https://godoc.org/google.golang.org/grpc/status to construct error responses, and set appropriate codes. The *Status type has a code that we can use (probably codes.NotFound in this case), and if necessary we can attach extra details that will be propagated in the response.

Cool it's exactly what we are already using so far on the lower daemon level inside the bblfshd, but without propagating this to the clients.

The thing is - we would still need to have error string parsing inside bblfshd, in order to get this condition as, as one can see in the second link, when this happens the only thing we get from Docker API underneath is errcode.ErrorCodeUnauthorized + an error message string.

If everyone thinks that it's a good idea to do so - I'll be happy to look a bit deeper and submit a PR exposing it later this Q.

\cc @dennwc

[creachadair]

The thing is - we would still need to have error string parsing inside bblfshd, in order to get this condition as, as one can see in the second link, when this happens the only thing we get from Docker API underneath is errcode.ErrorCodeUnauthorized + an error message string.

Unfortunately, that is sometimes the only available solution. We can write a function to do the translation to canonical codes in one place for this particular API, so that at least we have a seam for fixing regressions from upstream. The important thing from the client's perspective is that they should get reasonably standard error codes for things, and a message they could debug from. 🙂

Obviously parsing stderr is never ideal, but for messages that are generated programmatically it's usually at least possible to bound the set of possible responses. And of course we can always fall back to codes.Unknown as we do now, for cases we can't detect—that at least preserves the API on our side.

[creachadair]

Separately: An error like ErrCodeUnauthorized probably translates to PermissionDenied in the gRPC canonical codes. https://godoc.org/google.golang.org/grpc/codes has fairly detailed comments about the intended semantics of each code. The reason I mention this is that it can affect gRPC retry semantics.