mindflayer / python-mocket

a socket mock framework - for all kinds of socket animals, web-clients included
BSD 3-Clause "New" or "Revised" License
281 stars 42 forks source link

Errors with aiohttp 3.8.6 #207

Closed oschwald closed 11 months ago

oschwald commented 11 months ago

With the recent 3.8.6 release of aiohttp, we have started seeing errors such as this with mocket:

self = <ClientResponse(https://minfraud.maxmind.com/minfraud/v2.0/transactions/report) [None None]>
None

connection = Connection<ConnectionKey(host='minfraud.maxmind.com', port=443, is_ssl=True, ssl=None, proxy=None, proxy_auth=None, proxy_headers_hash=-4230833992692997272)>

    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]
                except http.HttpProcessingError as exc:
>                   raise ClientResponseError(
                        self.request_info,
                        self.history,
                        status=exc.code,
                        message=exc.message,
                        headers=exc.headers,
                    ) from exc
E                   aiohttp.client_exceptions.ClientResponseError: 400, message="Data after `Connection: close`:\n\n  b'null'\n     ^", url=URL('https://minfraud.maxmind.com/minfraud/v2.0/transactions/report')

../../.pyenv/versions/3.10.12/lib/python3.10/site-packages/aiohttp/client_reqrep.py:925: ClientResponseError

Although I am not 100% sure mocket is at fault, we haven't had any reports of errors in real use. You can see an example build here.

mindflayer commented 11 months ago

Hi @oschwald, I've just run twice the whole test-suite and it worked like a charm. I'll have a look at your specific error, anyway.

[...]

---------- coverage: platform linux, python 3.11.4-final-0 -----------
Name                                   Stmts   Miss  Cover   Missing
--------------------------------------------------------------------
mocket/__init__.py                         4      0   100%
mocket/async_mocket.py                     7      0   100%
mocket/compat.py                          18      0   100%
mocket/exceptions.py                       4      0   100%
mocket/mocket.py                         432     13    97%   103-105, 134, 216, 231-236, 251, 344, 487
mocket/mockhttp.py                       149      1    99%   69
mocket/mockredis.py                       52      0   100%
mocket/plugins/__init__.py                 0      0   100%
mocket/plugins/httpretty/__init__.py      66      0   100%
mocket/plugins/httpretty/core.py           2      0   100%
mocket/plugins/pook_mock_engine.py        47     45     4%   5-76
mocket/utils.py                           26      0   100%
--------------------------------------------------------------------
TOTAL                                    807     59    93%

=============================== 155 passed, 1 xpassed, 3 warnings in 10.85s ===============================

(.venv) ~/r/python-mocket (master|✔) $ pip freeze | grep aiohttp
aiohttp==3.8.6
mindflayer commented 11 months ago

A possible culprit could be https://github.com/aio-libs/aiohttp/pull/7336 Let me know if tests are passing with the rest of the Python versions >=3.8.

oschwald commented 11 months ago

I was testing on Python 3.11. I can try to come up with a smaller test case.

On Fri, Oct 20, 2023, 1:54 AM Giorgio Salluzzo @.***> wrote:

A possible culprit could be aio-libs/aiohttp#7336 https://github.com/aio-libs/aiohttp/pull/7336 Let me know if tests are passing with the rest of the Python version >=3.8 .

— Reply to this email directly, view it on GitHub https://github.com/mindflayer/python-mocket/issues/207#issuecomment-1772341924, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACECM4YAYX2KVTCQO6TS6DYAI35JAVCNFSM6AAAAAA6HSHKWOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGM2DCOJSGQ . You are receiving this because you were mentioned.Message ID: @.***>

mindflayer commented 11 months ago

I am sorry I did not notice the version from your first message logs. I was focusing on the pipeline you linked. If you manage to write down a snippet which fails that way, I'll definitely look into it.

oschwald commented 11 months ago

I looked into this more, and it turns out there was a bug in our test suite where we were returning the text "null" on 204s. The new version of llhttp in aiohttp was stricter about this. I apologize for the noise. Thanks for taking a look!