search

python-hyper / h2

HTTP/2 State-Machine based protocol implementation
https://h2.readthedocs.io/en/stable
MIT License
945 stars 152 forks source link

StreamIDTooLowError when trying to send headers into closed stream #1175

Open vmagamedov opened 5 years ago

vmagamedov commented 5 years ago

Hi!

I'm getting errors like this on the server-side:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/grpclib/server.py", line 310, in request_handler
    await method.func(stream)
  File "/usr/local/lib/python3.6/dist-packages/featureflags/server/rpc/service.py", line 51, in Exchange
    await self._queue.StoreStats.add(task, timeout=timeout)
  File "/usr/local/lib/python3.6/dist-packages/taskqueue/client/queue.py", line 21, in add
    await self._task_queue_stub.Add(task, timeout=timeout)
  File "/usr/local/lib/python3.6/dist-packages/grpclib/client.py", line 473, in __call__
    return await stream.recv_message()
  File "/usr/local/lib/python3.6/dist-packages/grpclib/client.py", line 286, in recv_message
    await self.recv_initial_metadata()
  File "/usr/local/lib/python3.6/dist-packages/grpclib/client.py", line 221, in recv_initial_metadata
    headers = await self._stream.recv_headers()
  File "/usr/local/lib/python3.6/dist-packages/grpclib/protocol.py", line 215, in recv_headers
    return await self.__headers__.get()
  File "/usr/lib/python3.6/asyncio/queues.py", line 167, in get
    yield from getter
concurrent.futures._base.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/h2/connection.py", line 585, in _get_or_create_stream
    return self.streams[stream_id]
KeyError: 48141

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/grpclib/server.py", line 325, in request_handler
    raise
  File "/usr/local/lib/python3.6/dist-packages/grpclib/server.py", line 220, in __aexit__
    status_message=status_message)
  File "/usr/local/lib/python3.6/dist-packages/grpclib/server.py", line 170, in send_trailing_metadata
    await self._stream.send_headers(headers, end_stream=True)
  File "/usr/local/lib/python3.6/dist-packages/grpclib/protocol.py", line 267, in send_headers
    end_stream=end_stream)
  File "/usr/local/lib/python3.6/dist-packages/h2/connection.py", line 763, in send_headers
    stream_id, AllowedStreamIDs(self.config.client_side)
  File "/usr/local/lib/python3.6/dist-packages/h2/connection.py", line 587, in _get_or_create_stream
    return self._begin_new_stream(stream_id, allowed_ids)
  File "/usr/local/lib/python3.6/dist-packages/h2/connection.py", line 454, in _begin_new_stream
    raise StreamIDTooLowError(stream_id, highest_stream_id)
h2.exceptions.StreamIDTooLowError: StreamIDTooLowError: 48141 is lower than 48143

This happens when client cancels stream with RST_STREAM, and on the server-side I have a logic which looks like this:

try:
    conn.send_headers(stream_id, [...])
except h2.exceptions.StreamClosedError:
    pass

And sometimes I'm getting StreamIDTooLowError exception instead of StreamClosedError, because this behavior depends on H2Connection.streams dictionary, where streams are removed nondeterministically.

I think that H2Connection._get_or_create_stream function should check not only H2Connection.streams but also H2Connection._closed_streams.

Of course I can refactor my code and check that stream is closed by myself, but I still think that there is a bug in H2Connection.send_headers method.

vmagamedov commented 5 years ago

Or it wasn't right to rely on StreamClosedError in the first place?