grpc-rust icon indicating copy to clipboard operation
grpc-rust copied to clipboard

Returning errors

Open andrenth opened this issue 6 years ago • 8 comments

Hi

Would you mind adding an example of the recommended way to return an error from a service? Is the GrpcStatus private by design (actually, the enum is public, but the grpc sub-module is private)? I tried to use SingleResponse::err(), but I can't construct a GrpcMessageError without GrpcStatus, unless I hard-code the status codes as integers.

Is there a more appropriate way for applications to return errors?

Thanks.

andrenth avatar Nov 21 '17 17:11 andrenth

GrpcStatus models internal GRPC errors, and yes, these are not supposed by used by library/protocol users.

GRPC model does not support user errors or exceptions. I guess the proper way to send errors is to reserve a field in your response object, e. g.

message MyResponse {
    bool success = 1;
    // proper response fields
}

However, for basic use cases like "everything's broken" grpc (and grpc-rust) supports "unknown" errors which can be accompanied by message string.

User of the library can return Error::Panic with message (simply panic).

stepancheg avatar Nov 22 '17 01:11 stepancheg

Thanks for the reply. It seems the Go library exposes the status codes and allows library users to specify them, e.g. via status.Errorf.

FWIW, I’ve found this issue with the grpc team recommended way to return errors.

andrenth avatar Nov 22 '17 02:11 andrenth

Not sure how authoritative this source is, but it contains examples for returning errors in various languages, all using the gRPC status codes: http://avi.im/grpc-errors/

Would you consider making GrpcStatus public?

andrenth avatar Nov 28 '17 18:11 andrenth

@andrenth I def agree with @stepancheg. A great pattern for error handling in grpc in general is to simply define a shared Error type which defines all of the core data needed about sharing error information between services. All response messages which could return an error will include this as the first element in the response struct. Can be defined in a shared proto file. E.G.:

message Error {
    string message = 1;
    uint32 status = 2; // Could directly map to HTTP status code.
    string code = 3; // Used for internal error tracking.
    map<string, string> meta = 4; // Additional info for client or whatever.
}

I've used this pattern quite successfully across multiple teams for pretty large scale microservice environments (all using grpc as the inter-service communication protocol). It works quite nicely with rust error checking patterns, as well as golang. You get the response message back always (like a Result). If the error field is not null, then handle the error, else use the response data. Network errors will be handled as a different beast entirely.

Similar patterns can be used for tracing. Define a shared Context struct. Will be expected in any request message &c.

Thoughts?

thedodd avatar Nov 29 '17 05:11 thedodd

This would certainly work. Have you tried using oneof in your response messages? Could be an interesting pattern:

message Response {
  oneof response {
    Error  error  = 1;
    Result result = 2;
  }
}

However, doesn't embedding errors in responses like this make handling a stream response a bit weird? In that case, do you return a single-element stream with the error? It seems it would make error handling in the client a bit unidiomatic in that case.

andrenth avatar Nov 29 '17 09:11 andrenth

@andrenth although, I think GrpcStatus shouldn't be used for signaling error by user, I'm OK with making it public (something like pub use grpc::GrpcStatus in lib.rs). Want to make a PR?

stepancheg avatar Nov 30 '17 00:11 stepancheg

@andrenth sorry for the super late response.

However, doesn't embedding errors in responses like this make handling a stream response a bit weird? ...

So, not necessarily. Whenever you receive a new message from the stream, check for the error field. If it is populated, then an error occurred. Simple as that. Same way you would handle errors with a unary response (barring the obvious differences).

Using oneof in this context does not seem logical, IMHO, because you may need to have more than one field in the non-error variant of the message. Instead, the contract can simply state that if the error is null, then the other "success" related fields must be populated according to the contract that the server upholds. And the inverse case follows the same line: if the error field is populated, then the other fields should not be populated.

Thoughts?

thedodd avatar Dec 07 '17 02:12 thedodd

I've been running into the same problem, and that is signaling a lack of authentication. grpc does have an error message for it, but I have no way to set it.

Code example:

impl Greeter for GreeterImpl {
    fn say_hello(&self, m: grpc::RequestOptions, req: HelloRequest) -> grpc::SingleResponse<HelloReply> {
        let auth = m.metadata.get("Authentication");
        match auth {
            None       =>
                grpc::SingleResponse::err(grpc::Error::GrpcMessage(grpc::GrpcMessageError {
                    grpc_status: /* grpc::grpc::GrpcStatus::Unauthenticated as i32 */ 16,
                    grpc_message: "No authentication provided".to_string()
                })),
            Some(auth) => {
                let mut r = HelloReply::new();
                let name = if req.get_name().is_empty() { "world" } else { req.get_name() };
                println!("greeting request from {}", name);
                r.set_message(format!("Hello {}", name));
                grpc::SingleResponse::completed(r)
            }
        }
    }
}

Also, I'm not even sure this returns the right thing, as the other end gets:

Err(Http(CodeError(InternalError)))

But for now, I support pub use grpc::GrpcStatus.

andreasdotorg avatar Dec 20 '17 14:12 andreasdotorg