grpc / grpc-dotnet

gRPC for .NET
Apache License 2.0
4.17k stars 767 forks source link

What is a better way to return fields validations #1029

Closed kelvinsand closed 4 years ago

kelvinsand commented 4 years ago

I had opened a issue some time ago, but I believe that it was not very clear.

I dont was able to participate in the discussion because I was really busy and I had to put that aside for a while I apologize to the topic participants for the delay in giving feedback.

I don't think I was very clear when I asked my question, but even then @lamLeX got really close to what I need. The treatment I need is about field validations. Whereas exceptions are costly and should only be used for exceptional things and not for flow control and Based on RFC 7807 and also in BadRequest message from google.rpc.error_details.proto. I built this example. I would like to know your opinion.

syntax = "proto3";

package user;

service UserService {
  rpc AddUser(User) returns (Reply);
}

message Reply { 
    oneof user_reply {
        User user = 1;
        Error error = 2;
    }
}

message User {
  string name = 1;
  int32 age = 2;
}

message Error {
    string title = 1;
    string detail = 2;
    // The status, which should be an enum value of [google.rpc.Code].
    int32 status = 3;

    message FieldViolation {
      string field = 1;
      repeated string messages = 2;
    }
    // I thought about using the map type here, but it is necessary that for each field a list of messages is returned
    repeated FieldViolation field_violations = 3;
}
chwarr commented 4 years ago

Your Error message is very similar to google.Rpc.Status. Since they're so similar, you may want to look at Google's documentation for how they typically model errors in gRPC APIs for ideas on how to model your failures.

Under this "google.Rpc convention", unsuccessful responses (non-OK responses) can include a serialized Status struct in the response's "grpc-status-details-bin" metadata (Implementations in Java, Go, Ruby, Python).

The google.Rpc.Status message has a repeated google.protobuf.Any details = 3; field. This is where additional structured messages with details about the failure can go, like your FieldViolation. My experience has been that the contents of the details field

Google has some common error detail messages, but you are free to make up your own messages, assuming the client and server agree.

Neither the gRPC.NET implementation nor the gRPC Core C# implementation have convenience methods for packing/unpacking this convention as of August 2020.


Also note: how to model failures in a ProtoBuf+gRPC service is distinct from how to fail a request in a given gRPC implementation. That is, ProtoBuf+gRPC have no concept of "exceptions": there are requests and responses. Responses can be successful or unsuccessful. Successful response have a payload of the message specified in the service definition. Unsuccessful responses have no payload.

Each gRPC implementation uses different APIs to expose the ability to send successful vs. unsuccessful responses. ProtoBuf+gRPC has no opinion on how design these APIs. E.g., gRPC.NET and gRPC Core C# both use the RpcException type. gRPC Core C++ has distinct APIs for completing a call with an OK vs. non-OK response.

If you are coming from a WCF background, there is no such thing as a fault contract. The current state of the art in ProtoBuf+gRPC is convention based.

kelvinsand commented 4 years ago

Thank you very much @chwarr for your return helped me to understand some things that were still a little confusing for me. I already knew the error handling defined by google and knew that it was not possible to use it with RPCException.

After a lot of research and reflection on the points mentioned, I ended up giving up the approach using oneof, I believe that it could hinder a more generic approach in the treatment of the answer. About my Error type It really is much better to use google.Rpc.Status which is already an official definition.

The best solution would be the support for google.Rpc.Status by RPCException (I am willing to help with the implementation, but I don't know where to start). While this is not available I will continue with something like this:

syntax = "proto3";

option csharp_namespace = "GrpcUser";

package UserApi;
import "google/rpc/status.proto";

service User {
  rpc Add(UserRequest) returns (UserResponse);
}

message UserResponse {
  string name = 1;
  int32 age = 2;
  google.rpc.Status status = 30;
}

message UserRequest {
  string name = 1;
  int32 age = 2;
}


I never worked with WCF but I was really confused about it. I believe that for always listening: "gRPC has an incoming and an outgoing message" and seeing RPCException being thrown made me confused. I had never looked at the code in depth, but after that information, I spent a few hours today analyzing it to better understand how it works. Thanks.

JunTaoLuo commented 4 years ago

Closing this as answered.

chwarr commented 3 years ago

I have a fully worked example of the grpc-status-details-bin convention in the https://github.com/chwarr/grpc-dotnet-google-rpc-status repository if this helps anyone else.