Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES/11100.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed spurious "Future exception was never retrieved" warnings for connection lost errors when the connector is not closed -- by :user:`bdraco`.

When connections are lost, the exception is now marked as retrieved since it is always propagated through other means, preventing unnecessary warnings in logs.
6 changes: 6 additions & 0 deletions aiohttp/client_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ def connection_lost(self, exc: Optional[BaseException]) -> None:
),
original_connection_error,
)
# Mark the exception as retrieved to prevent
# "Future exception was never retrieved" warnings
# The exception is always passed on through
# other means, so this is safe
with suppress(Exception):
self.closed.exception()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this maybe be handled by the set_exception() helper?

I assume that function is the one responsible for sending the exception elsewhere, so seems to me that it'd make sense for it to also suppress the unretrieved exception that it's handling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case it's the waiter below that is responsible for propagating the exception. We effectively have it in two places. There are other places where we don't so we can't make set_exception responsible as it doesn't know if it should suppress the warning or not


if self._payload_parser is not None:
with suppress(Exception): # FIXME: log this somehow?
Expand Down
19 changes: 19 additions & 0 deletions tests/test_client_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,22 @@ async def test_connection_lost_sets_transport_to_none(
proto.connection_lost(OSError())

assert proto.transport is None


async def test_connection_lost_exception_is_marked_retrieved(
loop: asyncio.AbstractEventLoop,
) -> None:
"""Test that connection_lost properly handles exceptions without warnings."""
proto = ResponseHandler(loop=loop)
proto.connection_made(mock.Mock())

# Simulate an SSL shutdown timeout error
ssl_error = TimeoutError("SSL shutdown timed out")
proto.connection_lost(ssl_error)

# Verify the exception was set on the closed future
assert proto.closed.done()
exc = proto.closed.exception()
assert exc is not None
assert "Connection lost: SSL shutdown timed out" in str(exc)
assert exc.__cause__ is ssl_error
Loading