Skip to content

Commit d067260

Browse files
authoredMar 16, 2025··
Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket (#10551)
<!-- Thank you for your contribution! --> ## What do these changes do? 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 We shouldn't raising `OSError` externally from this method as callers expect a `ClientError` ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10506 ## Checklist - [x] I think the code is well written - [x] Unit tests for the changes exist - [x] Documentation reflects the changes - [x] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is &lt;Name&gt; &lt;Surname&gt;. * Please keep alphabetical order, the file is sorted by names. - [x] Add a new news fragment into the `CHANGES/` folder * name it `<issue_or_pr_num>.<type>.rst` (e.g. `588.bugfix.rst`) * if you don't have an issue number, change it to the pull request number after creating the PR * `.bugfix`: A bug fix for something the maintainers deemed an improper undesired behavior that got corrected to match pre-agreed expectations. * `.feature`: A new behavior, public APIs. That sort of stuff. * `.deprecation`: A declaration of future API removals and breaking changes in behavior. * `.breaking`: When something public is removed in a breaking way. Could be deprecated in an earlier release. * `.doc`: Notable updates to the documentation structure or build process. * `.packaging`: Notes for downstreams about unobvious side effects and tooling. Changes in the test invocation considerations and runtime assumptions. * `.contrib`: Stuff that affects the contributor experience. e.g. Running tests, building the docs, setting up the development environment. * `.misc`: Changes that are hard to assign to any of the above categories. * Make sure to use full sentences with correct case and punctuation, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project.
1 parent 3b9bb1c commit d067260

File tree

3 files changed

+32
-1
lines changed

3 files changed

+32
-1
lines changed
 

‎CHANGES/10551.bugfix.rst

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
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`.

‎aiohttp/connector.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,10 @@ async def _wrap_create_connection(
11501150
# Will be hit if an exception is thrown before the event loop takes the socket.
11511151
# In that case, proactively close the socket to guard against event loop leaks.
11521152
# For example, see https://github.com/MagicStack/uvloop/issues/653.
1153-
sock.close()
1153+
try:
1154+
sock.close()
1155+
except OSError as exc:
1156+
raise client_error(req.connection_key, exc) from exc
11541157

11551158
def _warn_about_tls_in_tls(
11561159
self,

‎tests/test_connector.py

+27
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,33 @@ async def test_tcp_connector_closes_socket_on_error(
669669
await conn.close()
670670

671671

672+
async def test_tcp_connector_closes_socket_on_error_results_in_another_error(
673+
loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
674+
) -> None:
675+
"""Test that when error occurs while closing the socket."""
676+
req = ClientRequest("GET", URL("https://127.0.0.1:443"), loop=loop)
677+
start_connection.return_value.close.side_effect = OSError(
678+
1, "error from closing socket"
679+
)
680+
681+
conn = aiohttp.TCPConnector()
682+
with (
683+
mock.patch.object(
684+
conn._loop,
685+
"create_connection",
686+
autospec=True,
687+
spec_set=True,
688+
side_effect=ValueError,
689+
),
690+
pytest.raises(aiohttp.ClientConnectionError, match="error from closing socket"),
691+
):
692+
await conn.connect(req, [], ClientTimeout())
693+
694+
assert start_connection.return_value.close.called
695+
696+
await conn.close()
697+
698+
672699
async def test_tcp_connector_server_hostname_default(
673700
loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
674701
) -> None:

0 commit comments

Comments
 (0)
Please sign in to comment.