tonic
tonic copied to clipboard
hyperium
improper error handling with from h2 reason::cancel -> tonic::code::unkown
Bug Report
Version: latest
Platform: all (linux)
Crates
http_body, hyper, h2
Description
maybe Streaming::message need to handle h2 error? https://github.com/hyperium/tonic/blob/c62f382e3c6e9c0641decfafb2b8396fe52b6314/tonic/src/codec/decode.rs#L151 called from here https://github.com/hyperium/tonic/blob/c62f382e3c6e9c0641decfafb2b8396fe52b6314/tonic/src/client/grpc.rs#L193
I expected to see this happen: Code::Cancelled, message: stream no longer needed
Instead, this happened: code: Unknown, message: "error reading a body from connection: stream error received: stream no longer needed
Todo add minimal reproduce example
Are you able to share a minimal reproducible example?
not yet sorry i am not that skilled in rust. but if h2 will throws an error with reason::cancel and that is propagated by hyper's poll_inner fn at hyper/src/body/body.rs the downcast here will not catch it right? then it will be code::unknown mixed with hyper's message instead of code::cancelled which is confusing
Hyper errors have h2 errors as the "source" which should get picked up by tonic. At least I remember doing a bunch of testing around that last time we worked on it.
Hyper errors have h2 errors as the "source" which should get picked up by tonic. At least I remember doing a bunch of testing around that last time we worked on it.
maybe its a version issue let me double check, close the report for now
@davidpdrsn yep you are right hyper have h2 error as source. i have a quick question should we handle h2 error here since we are polling from the stream? i guess this is where the code known come from? will google how to repo bug with custom dependencies tmr. its way to late today.
https://github.com/hyperium/tonic/blob/c62f382e3c6e9c0641decfafb2b8396fe52b6314/tonic/src/codec/decode.rs#L151
Though we have get fn from_h2_error(err: &h2::Error), but this is not exhaust.
When set timeout 1us by tower service-builder.new().settimeout() layer then server build the tonic. The grpc server will timeout some req.
h2 log this:
http2 service errored: error from user's Service: request timed out,
and tonic rpc will get response : code: Unknow, message: transport error.
In this case you will need to downcast to the timeout error since you added that layer it looks like and tonic doesn't directly map those timeout errors on the server side iirc
Hi @LucioFranco , here's a rough implementation reproducing the error handling
https://github.com/behos/tonic-bidi-error
Ok I have a fix here for it https://github.com/hyperium/tonic/pull/1315 feel free to review it.