aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
14.63k stars 1.97k forks source link

Error message not always propagated on 3.9.4 #8395

Open ddeville opened 2 weeks ago

ddeville commented 2 weeks ago

Describe the bug

from aiohttp import web
from aiohttp.client_exceptions import ClientResponseError

async def handler(_: web.Request) -> web.Response:
    return web.Response(headers={"Custom": "x" * 10000})

app = web.Application()
app.add_routes([web.get("/", handler)])
client = await aiohttp_client(app)

try:
    await client.get("/")
except ClientResponseError as e:
    assert e.status == 400
    assert "Got more than 8190 bytes" in e.message
else:
    raise AssertionError("Expected ClientResponseError")

In 3.9.3 this would work and the ClientResponseError thrown would look like 400, message='Got more than 8190 bytes (10000) when reading Header value is too long.', url=URL('http://127.0.0.1:51163/'). Starting in 3.9.4 however an empty ClientResponseError seems to be raised 0, message='', url=URL('http://127.0.0.1:51193/').

I can see that the expected error message is thrown up the stack (specifically in aiohttp/client_proto.py) but the actual information about it is lost when it reaches aiohttp/client_reqrep.py.

    def data_received(self, data: bytes) -> None:
        self._reschedule_timeout()

        if not data:
            return

        # custom payload parser
        if self._payload_parser is not None:
            eof, tail = self._payload_parser.feed_data(data)
            if eof:
                self._payload = None
                self._payload_parser = None

                if tail:
                    self.data_received(tail)
            return
        else:
            if self._upgraded or self._parser is None:
                # i.e. websocket connection, websocket parser is not set yet
                self._tail += data
            else:
                # parse http messages
                try:
>                   messages, upgraded, tail = self._parser.feed_data(data)

../../.local/share/pyenv/versions/3.11.8/envs/mono-3.11.8/lib/python3.11/site-packages/aiohttp/client_proto.py:256:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
aiohttp/_http_parser.pyx:557: in aiohttp._http_parser.HttpParser.feed_data
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   ???
E   aiohttp.http_exceptions.LineTooLong: 400, message:
E     Got more than 8190 bytes (10000) when reading Header value is too long.

aiohttp/_http_parser.pyx:732: LineTooLong

The above exception was the direct cause of the following exception:

self = <ClientResponse(http://127.0.0.1:53889/) [None None]>
None

connection = Connection<ConnectionKey(host='127.0.0.1', port=53889, is_ssl=False, ssl=True, proxy=None, proxy_auth=None, proxy_headers_hash=None)>

    async def start(self, connection: "Connection") -> "ClientResponse":
        """Start response processing."""
        self._closed = False
        self._protocol = connection.protocol
        self._connection = connection

        with self._timer:
            while True:
                # read response
                try:
                    protocol = self._protocol
>                   message, payload = await protocol.read()  # type: ignore[union-attr]

../../.local/share/pyenv/versions/3.11.8/envs/mono-3.11.8/lib/python3.11/site-packages/aiohttp/client_reqrep.py:976:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <aiohttp.client_proto.ResponseHandler object at 0x104941240>

    async def read(self) -> _T:
        if not self._buffer and not self._eof:
            assert not self._waiter
            self._waiter = self._loop.create_future()
            try:
>               await self._waiter
E               aiohttp.http_exceptions.HttpProcessingError: 0, message:

../../.local/share/pyenv/versions/3.11.8/envs/mono-3.11.8/lib/python3.11/site-packages/aiohttp/streams.py:640: HttpProcessingError

To Reproduce

See repro case above.

Expected behavior

I expect ClientResponseError to have a 400 status rather than 0 and the message to be populated with the actual error.

Logs/tracebacks

See description above.

Python Version

3.11.8

aiohttp Version

Name: aiohttp
Version: 3.9.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author:
Author-email:
License: Apache 2
Location: /Users/damien/.local/share/pyenv/versions/mono-3.11.8/lib/python3.11/site-packages
Requires: aiosignal, attrs, frozenlist, multidict, yarl

multidict Version

Name: multidict
Version: 6.0.5
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /Users/damien/.local/share/pyenv/versions/mono-3.11.8/lib/python3.11/site-packages

yarl Version

Name: yarl
Version: 1.9.4
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache-2.0
Location: /Users/damien/.local/share/pyenv/versions/mono-3.11.8/lib/python3.11/site-packages

OS

macos sonoma 14.4.1

Related component

Client

Additional context

No response

Code of Conduct

webknjaz commented 2 weeks ago

Thanks for reporting the regression! It sounds like it may be a result of this improvement attempt: #8089.

Would you be able to submit a PR with just a regression test? I'll help whoever gets to fixing it in the future..

ddeville commented 2 weeks ago

Of course https://github.com/aio-libs/aiohttp/pull/8396