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)

Commits related to this issue

Most upvoted comments

+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

  1. that could happen today anyway, if the handler author doesn’t wrap those errors floating up from the library
  2. if the handler wants more explicit control over the Status, they always have that ability. they can just replace the library error with their own

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 using errors.As directly for me. It is so helpful if it’s available.

Based on what @mbyio mentioned above, another solution here could be:

package status

type StatError interface { // needs a better name; Error already defined
  error
  GRPCStatus() *Status
}

Usage:

_, err := runRPC(req)  // err has "Unwrap() error" returning a grpc status error
var unwrap status.StatError
if !errors.As(wrapped, &unwrap) {
   t.Fatalf("unable tounwrap wrapped error %v", wrapped)
}
st, ok := status.FromError(unwrap)
// use st safely if ok

This works more “naturally” with errors.As, but it seems much harder to use than:

_, err := runRPC(req)
st, ok := status.Unwrap(err)
// use st safely if ok

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.

if err := grpcClient.WithTransaction(ctx, func(ctx context.Context) error {
    // Do something
    _, err := grpcClient.DoSomething(ctx, &doSomethingRequest{})
    if err != nil {
    	return fmt.Errorf("error doing something: %w", err)
    }
    
    // Do something else
    _, err := grpcClient.DoSomethingElse(ctx, &doSomethingElseRequest{})
    if err != nil {
    	return fmt.Errorf("error doing something else: %w", err)
    }
    
    return nil
); err != nil {
    // Handle the error somehow
}

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:

resp, err := runRPC(ctx, req)
var grpcstatus interface{ GRPCStatus() *status.Status }
var code codes.Code 
if errors.As(err, &grpcStatus) {
    code = grpcstatus.GRPCStatus().Code()
}

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 the status package helper functions, if you can’t cast to something with GRPCStatus() *status.Status, try calling errors.As for GRPCStatus2(). 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 wrapped GRPCStatus() 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:

func HTTPError(ctx context.Context, mux *runtime.ServeMux, marshaler runtime.Marshaler, w http.ResponseWriter, r *http.Request, err error)

where the err error is produced by status.Errorf. It will nice to have the ability to unwrap this error / use errors.As and errors.Is to inspect the underlying error that’s being wrapped by status.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:

func HTTPError(ctx context.Context, mux *runtime.ServeMux, marshaler runtime.Marshaler, w http.ResponseWriter, r *http.Request, err error) {
	var jsonUnmarshalTypeErr *json.UnmarshalTypeError
	if errors.As(err, &jsonUnmarshalTypeErr) {
		w.WriteHeader(http.StatusBadRequest)
		w.Write([]byte("Payload invalid: type mismatch"))
	}
        ...
}

Just here to highlight my use case, thanks! 🙂