No error when streaming response is closed with RST_STREAM(NO_REASON)

[rumenov]

Bug Report

Version

for sure 0.13.x and most likely 0.14.x as well

Platform

linux

Description

When a client receives a response stream through a proxy (e.g., Envoy), the client fails to recognize when the proxy terminates the connection due to a timeout. Envoy closes the stream with RST_STREAM(NO_REASON) on timeout. The client just silently closes the stream without any error being surfaced. If I am not mistaken this even goes against the GPR spec

"Status must be sent in Trailers even if the status code is OK." and trailers must always be present

Please take a look at #2543

[LucioFranco]

I believe this is a bug that is kinda hard to fix since there is no real way to indicate why a task has gone away to then pass that to h2 that there is a timeout. I believe we are going to make sure this is solved in the grpc crate that we are working on cc @dfawley

[dfawley]

@rumenov - What is the actual behavior you're seeing from the tonic client when this occurs? Are you testing this with unary or bidi/server streaming?

A RST_STREAM error needs to be mapped to a grpc status code based on the error code per the gRPC-H2 spec. We shouldn't be mapping them all to INTERNAL. (Though obviously that's an improvement over OK.)

This is something we'll need to fix in tonic in order for the new grpc channel (which uses tonic) to have the right behavior here.

[rumenov]

Thank you for the replies @LucioFranco @dfawley !

@dfawley we have the following setup

1. replicated data server behind envoy
2. client makes server side streaming call
3. server replies immediately with success for the streaming call, but it takes longer than envoy timeout to start stream the body due to computation
4. Envoy hits the timeout and sends RST_STREAM(NO_REASON) to the client.
5. The client silently and successfully closes the stream with partial data or no data received.

This is very common setup for big data analytics platforms e.g. people using Arrow Flight RPC would observe the failure in similar situations.

A RST_STREAM error needs to be mapped to a grpc status code based on the error code per the gRPC-H2 spec.

Agreed.

So currently from the gRPC+H2 spec all 12 of the cases are broken by returning OK. If we blindly return Internal, as proposed on #2543, we have only in 4 broken cases. And in a way they are less broken because we still return an error instead of silently closing the stream.

Maybe a slightly different argument for returning internal. If I am not mistaken the spec reads that there must be trailers with grpc-status to guarantee the delivery of the call's terminal state. So if trailers are missing which is the case with the envoy timeout, then protocol error/internal makes sense as an error code.

[dfawley]

I see now that there is RST_STREAM error -> gRPC error mapping happening, and that you updated the comment in that function:

https://github.com/hyperium/tonic/pull/2543/changes#diff-41984bce8634964a0304ce98bb904d65bea7ba962a8d76452ed69e5b28f6387bR395

Any idea why this isn't happening / working correctly? I would definitely like to get to the bottom of that and figure out why it's not working as expected.

The rest of the PR you sent is probably almost perfect, and required regardless of the RST_STREAM mappingi problem. The one change would be: Go defaults to UNKNOWN for missing headers: https://github.com/grpc/grpc-go/blob/12e91ddb6df6150efa1dd13affc7e6cae7d12ff5/internal/transport/http2_client.go#L1477-L1478, which is probably this case from the status codes doc:

| Error parsing returned status | UNKNOWN | Client

So I think we should change the PR to use UNKNOWN in this case; otherwise I'm fine with it. And this seems like a pretty serious issue that could lead to data loss, so it should be considered high priority.

Thank you for finding this.