search

jvdwrf / axum-tonic

35 stars 9 forks source link

Handle `tonic::Status` errors like `tonic::transport::Server` does #2

Closed teohhanhui closed 2 years ago

teohhanhui commented 2 years ago

tonic::transport::Server uses a RecoverError middleware to convert tonic::Status errors into a http::Response:

https://github.com/hyperium/tonic/blob/919d28b2b96c7c803cec131a9e36e80d2b071701/tonic/src/transport/server/recover_error.rs

As far as I can tell, axum_tonic doesn't handle converting tonic::Status into response (I understand it's not possible to implement IntoResponse for tonic::Status).

And there's no way to use the above middleware since it's not public... I don't really understand what the middleware does, but it seems fairly complicated.

jvdwrf commented 2 years ago

Yes, for that reason there the GrpcStatus struct exists. This actually implements IntoResponse, but is itself just a wrapper around tonic::Status. There is a usage example in the docs.

teohhanhui commented 2 years ago

Yeah, I've seen that, but it can't be used in the gRPC service implementation. And I wonder if just adding our own middleware that converts tonic::Status to axum_tonic::GrpcStatus is enough? (If so, perhaps it can live in this project...)

My concern is as above about the complexity that might need to be handled... But again, I have yet to understand what's going on in the RecoverError middleware. Perhaps you have some better insight into that?

teohhanhui commented 2 years ago

Uhh, okay. So for simple cases it should be fine because of "Trailers-Only" response:

https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses

teohhanhui commented 2 years ago

Never mind. I have misunderstood how things work...

tonic::Status is already getting mapped to the correct http::Response here:

https://github.com/hyperium/tonic/blob/v0.8.0/tonic/src/server/grpc.rs#L328

which is ultimately called by the generated service...