Support grpc's WithDetails and errdetails for validator middleware

[F21]

Google has a bunch of error detail protobuf messages here: https://godoc.org/google.golang.org/genproto/googleapis/rpc/errdetails

I am currently performing validations by hand like so:

s, _ := status.Newf(codes.InvalidArgument, "invalid input").WithDetails(&errdetails.BadRequest{
	FieldViolations: []*errdetails.BadRequest_FieldViolation{
		{
			Field:       "SomeRequest.email_address",
			Description: "INVALID_EMAIL_ADDRESS",
		},
		{
			Field:       "SomeRequest.username",
			Description: "INVALID_USER_NAME",
		},
	},
})

return s.Err()

I am wondering if you guys can considering the following:

• Support validation of the whole request and return all bad fields in the error message.
• Support using errdetails.BadRequest to report which field is invalid.
• Allow customization of the description for each field violation.

[achew22]

First let me say I'm highly supportive of the idea. If you can find anywhere in the http api where they specify what to do with these explicitly I would LOVE to implement it. I'm just not sure how best to do that.

For validating the whole request. Right now we validate that your JSON can be unmarshalled into the message that you selected. I don't think it would be required to adopt this to improve those error messages.

For the rest, it seems like supporting transmitting errdetails as an object back in JSON is extremely desirable. Would you be interested in taking a swing at any of this?

[F21]

The best documentation I can find with handling the different GRPC errors are the guidelines for google cloud: https://cloud.google.com/apis/design/errors

I am currently following those guidelines in my own apps as they seem pretty well thought out to me.

The customization of error messages will need changes to the go-proto-validators project so it will probably be a good idea to start there. Ref: https://github.com/mwitkow/go-proto-validators/issues/28

Happy to take a stab at this once a spec is worked out and accepted by the maintainers 👍

[achew22]

Well how about that, they even have a http mapping in that spec! If you made it so that when you return a status on the trailers of a gRPC request you returned that I will approve it in a heartbeat (with tests).

The validator stuff seems like it would be best handled as an interceptor in your proxied grpc service that calls .Validate on the inbound message and triggers an error in that context.

[mwitkow]

@F21 there's a related https://github.com/mwitkow/go-proto-validators/issues/24 for the Validator annotation codegen.

I think the codegened valdiator could return a custom error that the Validator interceptor could understand and return as gRPC status errors.

I'm a little bit stamped at the moment, but it would be great to have this in and am happy to consult/review PRs.

[F21]

@mwitkow Thanks for the heads up! I haven't had the time to look into it as I currently have a lot on my plate. Let's see if we can get something into https://github.com/mwitkow/go-proto-validators first (implementing the corresponding bits in this repo should be pretty easy once that's done).

[achew22]

@mwitkow, I think there is a good chance you wouldn't even need to make the interceptor be generated. Armchair engineering here, but would it be possible to do a type assertion against an interface with Validate() error as the signature, then call it?

Note that this implements only grpc.UnaryServerInterceptor

type validatable interface {
  Validate() error
}

func(ctx context.Context, req interface{}, info *UnaryServerInfo, handler UnaryHandler) (interface{}, error) {
  if v, ok := req.(validatable); ok {
    err := v.Validate()
    if err != nil {
      return nil, grpc.Errorf(codes.InvalidArgument, "Error: %s", err)
    }
  }
  return handler(ctx, req)
}

[mwitkow]

@achew22 that's exactly how our internal validator interceptor that go-proto-validators works. But the interceptor needs to be custom to parse the specific error type of the validation framework in order to populate the WithDetails.

[eldad87]

@mwitkow, there is another issue, Validate() return an error string which is not useful. For now, It will be a good idea to expose the field/description of the failure.

[bwplotka]

Might be irrelevant with new Go proto, closing for now. Let us know if this is still relevant for v2 version, we can reopen.