grpc-go: Support error unwrapping (errors.As)
Several options:
- Implement
As
on*Status
so that users can do:
var s *status.Status
ok := errors.As(err, s)
// use s if ok
Note that *Status
does not implement error
, so this may be considered an anti-pattern. Another concern is that it would not be discoverable - how would users know to do this? Possibly through status
package documentation.
- Export
statusError
, though there is no other reason to do so. (I don’t like this.) - Add a
status.Unwrap(err) *Status
function to do unwrapping. This seems pretty reasonable, though it should theoretically be unnecessary. - Implement unwrapping support in
status.FromError()
. My only minor concern with this is that it would unwrap automatically, which may not be expected by users.
cc @jsm
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Reactions: 38
- Comments: 17 (7 by maintainers)
+1 on this. errors.Is() and errors.As() are becoming more and more common as the default way to handle error inspection, would be very useful to be able to pass a wrapped status error to status.FromError() or errors.As().
There was a PR for this which was declined and closed: https://github.com/grpc/grpc-go/pull/4091
I don’t really understand the commentors comment about why it was declined. The argument was something like, if the handler is returning an error without attempting to transform it to a specific Status, then the author of the controller will be expecting this error to result in an Unknown status code. If later, a library decides to implement GRPCStatus() in order to return a different status code, then the controller author could be surprised that their codes are changing.
But
But today, when they do that, they lose all the rich context information attached to the original error. If they are using an intercepter for transaction logging, that middleware will never get to see the original error object, and all that interesting context will be lost. Even if I use a custom error implementation which holds the original error, and implement GRPCStatus(), I can’t then wrap that error any further, because grpc won’t use
errors.As()
to unwrap down to by Status holder.After so many years of errors.Is() and errors.As() in the ecosystem, it’s very surprising behavior that grpc doesn’t use errors.As() to find GRPCStatus() implementors.
The status package provides high level functions for gRPC status and its error, so
status.Unwrap
seems very good to handle wrapped errors than usingerrors.As
directly for me. It is so helpful if it’s available.Based on what @mbyio mentioned above, another solution here could be:
Usage:
This works more “naturally” with
errors.As
, but it seems much harder to use than:I definitely think this feature would be helpful for gRPC clients.
Say you have a gRPC client that is capable of making requests transactionally. If any of the requests return errors, it’d be nice to let the client wrap those errors so that it can provide more context about which request was executing when the server returned an error.
Ex.
I’ll probably use something similar to @FUSAKLA’s approach here until this package supports some form of sustainable error wrapping, which I imagine should follow the Go community settling on an idiomatic approach to the problem. Probably a good idea to let the dust settle after the Go 1.13 change.
@dfawley for reference.
@dfawley
Yeah without making a breaking change, I’m not sure you need to make any code changes. It is trivial to fetch the status object (and associated info) manually:
If you add new functions or make any other changes, I think it will be confusing, because people might think GRPC is using those functions to retrieve the status. In other words, IMO, the
status
package should not encourage unwrapping if it isn’t going to use that for the actual GRPC error response.An alternative could be to define a new interface for errors to implement. Something like
interface{ GRPCStatus2() *status.Status}
. In thestatus
package helper functions, if you can’t cast to something withGRPCStatus() *status.Status
, try callingerrors.As
forGRPCStatus2()
. However, that is starting to get complicated, and you’d have to add a new error type people have to opt into to avoid breaking backwards compatibility.I think the best way to address this without a breaking change would be to add interceptors to grpc_middleware that simplify this error conversion process (which for most casual users will be very confusing), and then document how to set them up in this package’s docs. Similar to what @FUSAKLA did for that one interceptor. Maybe one interceptor for
pkg/errors
since that is extremely popular, and one for the stdlib package, and users can use both if they want to cover all their bases. The interceptors would search for a wrappedGRPCStatus()
and pull it up to the top level.As a user I’d strongly prefer a breaking change here, especially because the breakage is easy to undo with an interceptor if people want to opt-out. When monitoring middleware get involved, keeping everything in agreement about how to fetch GRPC errors gets complicated. But I recognize breaking is not always possible.
I use an interceptor to check for aborted/conflict grpc errors and implement an automatic retry. However I have a lot of code in the server itself that is doing wrapping at each level to implement a form of stack traces.
Being able to unwrap an error would help me with this.
status.Code()
method checks is already doing the interface check, why not just do an unwrap on that?@jace-ys - this issue is about unwrapping errors to find grpc status errors within them. It sounds like what you’re asking for is the ability to do the opposite: wrap arbitrary errors inside status errors. This is actually ~possible by using the error details, although admittedly not in a way that works transparently with Go’s error wrapping. Unfortunately, we are not considering this kind of capability, because it would result in a status error that cannot be transmitted over the wire from a server to a client identically. (Errors are arbitrary types which may not be serializable.)
+1 on this feature request. Not sure how common my use case is, but would be nice to have this.
I use the grpc-gateway as a gRPC client to expose my gRPC service as a REST API. The gateway allows you customize the HTTPError to return a custom HTTP response from the gateway based on the error.
HTTPError has the following signature:
where the
err error
is produced bystatus.Errorf
. It will nice to have the ability to unwrap this error / useerrors.As
anderrors.Is
to inspect the underlying error that’s being wrapped bystatus.Errorf
to perform custom handling.In my case, I would like to be able to do this to handle json unmarshaling errors that occurs in the gateway:
Just here to highlight my use case, thanks! 🙂