Skip to content

Commit a3c36ad

Browse files
[PR #10551/d067260d backport][3.12] Re-raise OSError as ClientConnectionError when failing to explicitly close connector socket (#10562)
**This is a backport of PR #10551 as merged into master (d067260).** <!-- 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. Co-authored-by: J. Nick Koston <[email protected]>
1 parent 25c9ab8 commit a3c36ad

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
async def _wrap_existing_connection(
11561159
self,

tests/test_connector.py

+27
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,33 @@ async def test_tcp_connector_closes_socket_on_error(
640640
await conn.close()
641641

642642

643+
async def test_tcp_connector_closes_socket_on_error_results_in_another_error(
644+
loop: asyncio.AbstractEventLoop, start_connection: mock.AsyncMock
645+
) -> None:
646+
"""Test that when error occurs while closing the socket."""
647+
req = ClientRequest("GET", URL("https://127.0.0.1:443"), loop=loop)
648+
start_connection.return_value.close.side_effect = OSError(
649+
1, "error from closing socket"
650+
)
651+
652+
conn = aiohttp.TCPConnector()
653+
with (
654+
mock.patch.object(
655+
conn._loop,
656+
"create_connection",
657+
autospec=True,
658+
spec_set=True,
659+
side_effect=ValueError,
660+
),
661+
pytest.raises(aiohttp.ClientConnectionError, match="error from closing socket"),
662+
):
663+
await conn.connect(req, [], ClientTimeout())
664+
665+
assert start_connection.return_value.close.called
666+
667+
await conn.close()
668+
669+
643670
async def test_tcp_connector_server_hostname_default(
644671
loop: Any, start_connection: mock.AsyncMock
645672
) -> None:

0 commit comments

Comments
 (0)