stream.end() on stream closed from other side leaves protocol in a broken state and raises h2 exception

[mk-fg]

Hi,

I'm not very familiar with gRPC and grpclib, so feel free to correct me on anything below, as I might be misunderstanding what's going on here.

Have following issue when gRPC stream gets closed:

Traceback (most recent call last):
  File "/root/.local/lib/python3.8/site-packages/etcd3aio/watch.py", line 112, in sender_task
    await stream.end()
  File "/root/.local/lib/python3.8/site-packages/grpclib/client.py", line 289, in end
    await self._stream.end()
  File "/root/.local/lib/python3.8/site-packages/grpclib/protocol.py", line 461, in end
    self._h2_connection.end_stream(self.id)
  File "/root/.local/lib/python3.8/site-packages/h2/connection.py", line 883, in end_stream
    frames = self.streams[stream_id].end_stream()
  File "/root/.local/lib/python3.8/site-packages/h2/stream.py", line 956, in end_stream
    self.state_machine.process_input(StreamInputs.SEND_END_STREAM)
  File "/root/.local/lib/python3.8/site-packages/h2/stream.py", line 129, in process_input
    return func(self, previous_state)
  File "/root/.local/lib/python3.8/site-packages/h2/stream.py", line 348, in send_on_closed_stream
    raise StreamClosedError(self.stream_id)
h2.exceptions.StreamClosedError: 5

If I understand correctly, stream is already closed from the other side (etcd daemon there logs error, so I assume does some kind of closing too), and attempting to "end" it via grpclib fails, as it tries to send something gRPC-related there, which it can't do in this state.

This looks like a bug, as underlying h2 exception slips through the stack instead of grpclib's StreamTerminatedError or something like that, if anything should be raised here at all.

And this also does not finish closing the stream, leading to following error, I assume when GC collects the stream, if ignored:

Traceback (most recent call last):
  File "/root/.local/lib/python3.8/site-packages/etcd3aio/watch.py", line 114, in sender_task
    except h2.exceptions.StreamClosedError: pass
  File "/root/.local/lib/python3.8/site-packages/grpclib/client.py", line 531, in __aexit__
    raise exc_val
  File "/root/.local/lib/python3.8/site-packages/grpclib/client.py", line 521, in __aexit__
    await self._maybe_finish()
  File "/root/.local/lib/python3.8/site-packages/grpclib/client.py", line 497, in _maybe_finish
    await self.recv_trailing_metadata()
  File "/root/.local/lib/python3.8/site-packages/grpclib/client.py", line 438, in recv_trailing_metadata
    raise ProtocolError('Outgoing stream was not ended')
grpclib.exceptions.ProtocolError: Outgoing stream was not ended

I.e. there doesn't seem to be a way to handle such case cleanly in grpclib-using application at the moment, as stream.end() does not work and yet not ending the stream is a ProtocolError.

Seem to be happening here when communicating with etcd using grpclib-0.3.2 and grpclib-0.4.0rc1 installed from current git 87cc902 via pip install --user https://github.com/vmagamedov/grpclib/.

[vmagamedov]

The reason you get these errors is because "your" code handles StreamTerminatedError inside Method.open() context-manager without re-raising it:

https://github.com/hron/etcd3aio/blob/6857cc2811d4eb8dc1f6e92145985c27ec9d51b4/etcd3aio/watch.py#L109-L110

You get the first exception because you try to continue work with a closed stream, we probably received RST_STREAM frame from a server, this ultimately closes the stream and we're not allowed to send anything on it, this is the final state.

You get the second exception because you don't re-raise original exception and grpclib thinks that there was no errors and it complains that you're not finished the call properly.

Here is the corresponding docs: https://grpclib.readthedocs.io/en/latest/errors.html#client-side

[mk-fg]

Ah, I see, didn't think it could be context's responsibility to handle these.
Will see about adding a patch there to handle GRPCError outside of context instead.

Thank you for detailed explaination, and apologies for a bogus report.