AssertionError: "assert transport is not None" in aiohttp.web_request.BaseRequest.__init__

[kozlovsky]

Describe the bug

We are using aiohttp 3.8.3 and occasionally encounter the following error:

Traceback (most recent call last):, 
  File "aiohttp\web_protocol.py", line 505, in start, 
  File "aiohttp\web_app.py", line 446, in _make_request, 
  File "aiohttp\web_request.py", line 811, in __init__, 
  File "aiohttp\web_request.py", line 188, in __init__, 
AssertionError: transport is not None

As I understand it, the error occurs due to the following sequence of actions:

First, the web_protocol.RequestHandler.start method is executed:

    async def start(self) -> None:
        ...
        while not self._force_close:
            if not self._messages:
                try:
                    # wait for next request
                    self._waiter = loop.create_future()
                    await self._waiter  # <-- I suspect CancelledError is raised here because the `start` coroutine itself is cancelled
                except asyncio.CancelledError:
                    break  # <-- the code suppresses CancelledError and continue execution while the transport is already closed 
                finally:
                    self._waiter = None

            message, payload = self._messages.popleft()
            ...
            request = self._request_factory(message, payload, self, writer, handler)  # <-- here we have AssertionError because the transport is already closed

The await self._waiter line is wrapped in a try-except block that suppresses CancelledError. However, it is generally considered bad practice to catch CancelledError without re-raising it because await something() can raise CancelledError for two entirely different reasons:

1. The something() that being awaiting was cancelled;
2. The current coroutine itself was canceled, and the exception was thrown into it.

I believe the second scenario is what we are experiencing in this error. The connection was closed from the client side; the transport was set to None, and web_protocol.RequestHandler.start itself was canceled. However, the CancelledError was mistakenly suppressed, assuming it was related to self._waiter. As a result, web_protocol.RequestHandler.start continued executing, and when calling request = self._request_factory(...), an assertion error occurred because the transport had already been closed.

As a quick fix, it is possible to check if self.transport is None before calling self._request_factory(...), and, if True, raise CancelledError that was swallowed previously (not sure if this fix is completely correct though).

To Reproduce

Probably the error arises when the connection is closed from the client side before the server is able to start the request handling.

Expected behavior

When a connection is closed from the client side, the code should work without any AssertionError exceptions

Logs/tracebacks

Traceback (most recent call last):, 
  File "aiohttp\web_protocol.py", line 505, in start, 
  File "aiohttp\web_app.py", line 446, in _make_request, 
  File "aiohttp\web_request.py", line 811, in __init__, 
  File "aiohttp\web_request.py", line 188, in __init__, 
AssertionError

Python Version

3.8.4

aiohttp Version

3.8.3

multidict Version

irrelevant

yarl Version

irrelevant

OS

Windows, Linux

Related component

Server

Additional context

No response

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[Dreamsorcerer]

However, the CancelledError was mistakenly suppressed, assuming it was related to self._waiter.

No, that doesn't suppress it, it breaks. So, on cancellation, it would skip to the bottom of the function and potentially close the transport before cleanly ending the task.

As a quick fix, it is possible to check if self.transport is None before calling self._request_factory(...), and, if True, raise CancelledError that was swallowed previously (not sure if this fix is completely correct though).

Because of the above, this won't make any difference (edit: Or maybe it would, given the next comment).

We've been looking at a potentially related issue though: #6978 (comment)

[Dreamsorcerer]

I think it's actually caused by the same issue. On connection_lost, transport is set to None:
https://github.com/aio-libs/aiohttp/blob/3.8/aiohttp/base_protocol.py#L67

But, in 3.8, we no longer cancel the handler.
So, I think in this case, we are losing the connection while waiting for data from the client, while in the other issue we are losing the connection while maybe waiting on the endpoint handler to complete.

Fancy taking a go at creating a test to reproduce this in the CI?

[tfrokt]

Same issue in aiohttp==3.8.5

Task exception was never retrieved
future: <Task finished name='Task-419' coro=<RequestHandler.start() done, defined at /usr/local/lib/python3.10/dist-packages/aiohttp/web_protocol.py:462> exception=AssertionError()>
Traceback (most recent call last):
File "/usr/local/lib/python3.10/dist-packages/aiohttp/web_protocol.py", line 505, in start
request = self._request_factory(message, payload, self, writer, handler)
File "/usr/local/lib/python3.10/dist-packages/aiohttp/web_app.py", line 446, in _make_request
return _cls(
File "/usr/local/lib/python3.10/dist-packages/aiohttp/web_request.py", line 811, in __init__
super().__init__(*args, **kwargs)
File "/usr/local/lib/python3.10/dist-packages/aiohttp/web_request.py", line 188, in __init__
assert transport is not None
AssertionError