gRPC support for application error encoding and decoding through metadata

[ravlio]

It is would be very nice to make application error handlers for gRPC as same as in http transport realization. I need to forward custom error with code and message and some other data from server to client and I have no other ideas then to put it all to metadata. I think that it is possible to do by putting error handler in server after decoder and invoker and client after invoker in the manner of http transport do.

[basvanbeek]

I'm not sure I understand the problem you're trying to solve, can you give an example? If you have errors that need to be transported from server to client it sounds like those should be part of the gRPC response messages that you define in your proto file as they concern the business domain and not the transport domain.

The gRPC client and gRPC server do expose the same before and after hooks as the HTTP transport, so you should be able to handle non business domain items appropriately. This is where metadata concerns can be handled. gRPC metadata is in that sense the equivalent of HTTP headers.

[ravlio]

I'm in doubt how to send custom error with status and message from server to client, yes. Some people suggesting to include it in metadata (https://groups.google.com/forum/#!topic/grpc-io/9yCvsQWTP9w) Someone says to include it in return message with https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto
But I can't understand, if Google promotes canonical errors in https://cloud.google.com/appengine/docs/admin-api/reference/rpc/google.rpc why they did not use themselves? https://cloud.google.com/appengine/docs/admin-api/reference/rpc/google.appengine.v1beta5#listservicesresponse

So my question is what is better approach in managing of application errors? Thanks.

[ravlio]

Thank you for refactoring of examples, things getting more clear, I have found grpc error handler in addSvc with custom errors. But if I provoke some error like ErrTwoZeroes the server goes down and log error three times in a row. Looks more like unrecoverable protocol fault then regular handled error when I simply can check error with if err != nil && err.(type) == ErrMyCustomError on client.

I can do all the validations on client side but there is always the chance of errors. Any thoughts on this? Best practices? I want to mention that error may occur not only in service but often in request decoder function.

[basvanbeek]

That's indeed an error we've identified as well... in the decode function we forgot to test for a nil reply.. we're going to update the examples again to provide better insight into the error handling options you have.

I personally like having explicit error payloads in all return messages as it gives more flexibility towards the error payload you'd like to transport. It also makes it easy to distinguish transport errors from business logic errors and have the transport errors count towards your circuit breaker, while common user input errors not identifying service issues do not.

[peterbourgon]

I've fixed the crashing problem, a simple bug, mea culpa.

I've submitted #287 to move addsvc errors into the response types, which will at least partially address your questions. Please take a look?

[ravlio]

Thank for such a quick feedback. Interesting approach. Never seen it before (I have look at etcd, kubernates, google bigdata APIs), but have to used it in project.

The last open unresolved question for me is how to catch errors earlier, not from endpoint, but from request decoder. But it can't be done because decoder returns error code which handled internally in:

// from transport/grpc/server.go

request, err := s.dec(grpcCtx, req)
    if err != nil {
        s.logger.Log("err", err)
        return grpcCtx, nil, BadRequestError{err}
    }

I have do many validations while request decoding like missing fields checking, format errors, etc. Any thoughts about this? Where you think this is need to be placed? Maybe validator need to be separated from decoder and called directly from each endpoint before other business logic? Or maybe wrapped in middleware like instrumenting or logging? Err in response struct is OK, but err in request struct is overkill I think :)

In HTTP transport decode error can be handled. Lets think how it can be done in RPC.

[basvanbeek]

if you need to validate payloads that are regarded as valid by your gRPC definition, it sounds like you either need to tighten your proto definition file or it is probably validation of business logic and not so much gRPC specific.

In that latter case I would suggest handling inside your service implementation or as service middleware as probably those validations have little to do with the underlying transport and everything with generic input validation and sanitization.

The one exception being incorrect / ambiguous translations between your protobuf payloads and go kit request_response payloads; those would belong in your codecs.

[basvanbeek]

@ravlio are there still items unclear regarding this issue?

[ravlio]

@basvanbeek Everything is great. We have already integrate this approach in our project. For request decode error handling we use approach same as for response — just have Error field in Request struct and then catch it in endpoint.

func MakeCreateCampaignEndpoint(svc common.Service) endpoint.Endpoint {
    return func(ctx context.Context, request interface{}) (interface{}, error) {
        req := request.(msg.CreateCampaignRequest)

        if req.Error != nil {
            return msg.CreateCampaignResponse{
                Error:    err,
            }, nil
        }

        id, err := svc.CreateCampaign(ctx, req.Campaign)

        return msg.CreateCampaignResponse{
            Campaign: &msg.Campaign{ID: id},
            Error:    err,
        }, nil
    }
}