Your interpretation is correct.As for the duplicate error checking, yeah, it is not the most convenient thing but given all the other constraints, we settled on it.
>>Do you literally do this for all requests, or just ones that may have more complex error handling logic We already have an existing rpc system that allowed for defining a per rpc set of errors and error object and most rpcs already did use that functionality. Grpc came as an additional capability. So yes, we do literally do it for all the rpcs. However, if a system was designed from ground up/grpc only, I can see not wanting to do this for all rpcs. Moreover, all the fields in Proto3 are optional so the client has to always take that into account anyway (meaning that google.rpc.status may or may not be present). Thanks. On Fri, Sep 15, 2017 at 7:09 AM, Evan Jones <[email protected]> wrote: > Interesting! Thanks for sharing. Just so I'm clear, this means you only > use the gRPC status to mean "did the RPC get responded to correctly or > not", and the "application errors" are embedded in the response struct. > This seems to be a reasonable approach. Do you find that the duplicate > error checking (both the gRPC code, and the application code) is not > annoying? Do you literally do this for all requests, or just ones that may > have more complex error handling logic? > > (For clarity, this is basically what I meant by "redefining success": The > *RPC* is successful, even if the application request was not). > > Thanks! > > Evan > > > > On Thursday, September 14, 2017 at 12:52:28 PM UTC-4, Arpit Baldeva wrote: >> >> I work in C++ but I think the strategy we have can be adopted in any >> language. >> >> We use Grpc status code for the errors for very limited system level >> failures. This way, the mapping from our application logic to grpc status >> codes is limited. An incentive for doing this is streaming rpcs where we >> may not want to close down the stream because one of the request had some >> kind of an error. If a grpc status other than OK is returned, the behavior >> is to close the stream. >> >> Then, we add google.rpc.status (https://github.com/googleapis >> /googleapis/blob/master/google/rpc/status.proto ) to each response >> message. This way, client can check for an error code in a context specific >> manner for the rpc. This should get rid of the string typo problem you >> have. You can also provide an error message and even more details using Any >> message. >> >> On client, it becomes a two level check. While not as simple as I'd like >> it to be, this is the best design we could think of: >> >> if (status.ok()) // This is grpc status >> { >> if (reply.status().code() == 0) // This is the >> google::rpc::status proto embedded in the response message >> { >> outputFile << "Rpc call succeeded. Response is" << >> std::endl; >> >> } >> else >> { >> outputFile << "Rpc call failed. Error details are" << >> std::endl; >> >> } >> } >> else >> { >> outputFile << "Rpc call failed." << std::endl; >> outputFile << "System status code (" << >> gRPCStatusCodeToString(status.error_code()) << "|" << >> status.error_code() << ")\n" << "System status message (" << >> status.error_message() << ")" << std::endl; >> } >> >> >> >> On Wednesday, September 13, 2017 at 5:16:02 PM UTC-7, Francis Chuang >> wrote: >>> >>> These are all very good points. I've looked at etcd3 as well as a few >>> other projects using GRPC and they just rely on the GRPC status codes. >>> >>> I guess the root of the problem is probably the granularity of the GRPC >>> status codes. For example, in the service implementation, I might have >>> multiple errors returned as INVALID_ARGUMENT. However, it is not possible >>> to signal which field is invalid in a given request. >>> >>> I think the only way to do this is to use WithDetails() and the >>> BadRequest protobuf message type. >>> >>> I have decided on the following strategy: >>> - Do not use WithDetails() if possible. >>> - If the client really needs the extra granularity (to implement >>> different behaviors), then use WithDetails(). >>> >>> I think the code is easier to manage this way, however, there's a fair >>> bit of mental overhead/coupling to decide whether the client will require >>> this granularity. >>> >>> In addition, there is still 1 problem I have not successfully solved: >>> For extra granularity, I am returning strings like >>> "EMAIL_ADDRESS_NOT_FOUND" as the BadRequest's Description as part of >>> WithDetails(). However, this requires a fair bit of bookkeeping in the >>> client. For example, there is no way to signal that the string has changed >>> or has been deprecated without a lot of manual documentation. In addition, >>> there is no nice way to guard against typos when using these strings in the >>> clients. >>> >>> >>> -- > You received this message because you are subscribed to a topic in the > Google Groups "grpc.io" group. > To unsubscribe from this topic, visit https://groups.google.com/d/ > topic/grpc-io/k6QFNxWDmv0/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/grpc-io. > To view this discussion on the web visit https://groups.google.com/d/ > msgid/grpc-io/f55a1b62-297f-4c55-9fdb-b649cda400f0%40googlegroups.com > <https://groups.google.com/d/msgid/grpc-io/f55a1b62-297f-4c55-9fdb-b649cda400f0%40googlegroups.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/grpc-io. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CAL2p%3D0nT4iJmNObgwJWWfUxSp-mO8brOEKnRjcByVHOdjpNxRw%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
