From 0444be0c13f6966668d00bce635ddd533c289e88 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 14 Mar 2025 13:58:20 -1000 Subject: [PATCH 1/4] Reraise OSError as ClientConnectionError when failing to explictly close connector socket This is a followup to #10464 to handle the case where socket.close() can also raise. This matches the logic we have in aiohappyeyeballs: https://github.com/aio-libs/aiohappyeyeballs/blob/e3bd5bdf44f5d187802de6dcb08d27e1ca6da048/src/aiohappyeyeballs/impl.py#L227 fixes #10506 --- aiohttp/connector.py | 5 ++++- tests/test_connector.py | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 8a3f1bcbf2b..c1e18566b86 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -1143,7 +1143,10 @@ async def _wrap_create_connection( # Will be hit if an exception is thrown before the event loop takes the socket. # In that case, proactively close the socket to guard against event loop leaks. # For example, see https://github.com/MagicStack/uvloop/issues/653. - sock.close() + try: + sock.close() + except OSError as exc: + raise client_error(req.connection_key, exc) from exc def _warn_about_tls_in_tls( self, diff --git a/tests/test_connector.py b/tests/test_connector.py index 076ed556971..4bfd2b374b0 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -669,6 +669,33 @@ async def test_tcp_connector_closes_socket_on_error( await conn.close() +async def test_tcp_connector_closes_socket_on_error_results_in_another_error( + loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock +) -> None: + """Test that when error occurs while closing the socket.""" + req = ClientRequest("GET", URL("https://127.0.0.1:443"), loop=loop) + start_connection.return_value.close.side_effect = OSError( + 1, "error from closing socket" + ) + + conn = aiohttp.TCPConnector() + with ( + mock.patch.object( + conn._loop, + "create_connection", + autospec=True, + spec_set=True, + side_effect=ValueError, + ), + pytest.raises(aiohttp.ClientConnectionError, match="error from closing socket"), + ): + await conn.connect(req, [], ClientTimeout()) + + assert start_connection.return_value.close.called + + await conn.close() + + async def test_tcp_connector_server_hostname_default( loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock ) -> None: From c097a83fe976fd750b58a5c5b276a5e86f3b724b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 14 Mar 2025 14:02:57 -1000 Subject: [PATCH 2/4] changelog --- CHANGES/10551.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/10551.bugfix.rst diff --git a/CHANGES/10551.bugfix.rst b/CHANGES/10551.bugfix.rst new file mode 100644 index 00000000000..c4f3704f96a --- /dev/null +++ b/CHANGES/10551.bugfix.rst @@ -0,0 +1 @@ +The connector now raises :exc:`aiohttp.ClientConnectionError` instead of :exc:`OSError` when failing to explicitly close the socket after ``loop.create_connection`` fails -- by :user:`bdraco`. From f3ce6a8cae9b48c457c7e0bfedffb88d7e320c70 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 14 Mar 2025 14:04:02 -1000 Subject: [PATCH 3/4] changelog --- CHANGES/10551.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/10551.bugfix.rst b/CHANGES/10551.bugfix.rst index c4f3704f96a..06040b5ef6d 100644 --- a/CHANGES/10551.bugfix.rst +++ b/CHANGES/10551.bugfix.rst @@ -1 +1 @@ -The connector now raises :exc:`aiohttp.ClientConnectionError` instead of :exc:`OSError` when failing to explicitly close the socket after ``loop.create_connection`` fails -- by :user:`bdraco`. +The connector now raises :exc:`aiohttp.ClientConnectionError` instead of :exc:`OSError` when failing to explicitly close the socket after :py:func:`asyncio.loop.create_connection` fails -- by :user:`bdraco`. From a48e406de927160df3637d71005b2c74e6759fa7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 14 Mar 2025 14:13:03 -1000 Subject: [PATCH 4/4] fix docs mapping --- CHANGES/10551.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/10551.bugfix.rst b/CHANGES/10551.bugfix.rst index 06040b5ef6d..8f3eb24d6ae 100644 --- a/CHANGES/10551.bugfix.rst +++ b/CHANGES/10551.bugfix.rst @@ -1 +1 @@ -The connector now raises :exc:`aiohttp.ClientConnectionError` instead of :exc:`OSError` when failing to explicitly close the socket after :py:func:`asyncio.loop.create_connection` fails -- by :user:`bdraco`. +The connector now raises :exc:`aiohttp.ClientConnectionError` instead of :exc:`OSError` when failing to explicitly close the socket after :py:meth:`asyncio.loop.create_connection` fails -- by :user:`bdraco`.