diff --git a/CHANGES/9883.bugfix.rst b/CHANGES/9883.bugfix.rst new file mode 100644 index 00000000000..3ffb8361448 --- /dev/null +++ b/CHANGES/9883.bugfix.rst @@ -0,0 +1 @@ +Fixed improperly closed WebSocket connections generating an unhandled exception -- by :user:`bdraco`. diff --git a/aiohttp/_websocket/reader_py.py b/aiohttp/_websocket/reader_py.py index 04d654f8bf3..aa4b6ba2704 100644 --- a/aiohttp/_websocket/reader_py.py +++ b/aiohttp/_websocket/reader_py.py @@ -69,6 +69,9 @@ def __init__( self._get_buffer = self._buffer.popleft self._put_buffer = self._buffer.append + def is_eof(self) -> bool: + return self._eof + def exception(self) -> Optional[Union[Type[BaseException], BaseException]]: return self._exception diff --git a/tests/test_client_ws_functional.py b/tests/test_client_ws_functional.py index bcb1669c3e5..d42582f0078 100644 --- a/tests/test_client_ws_functional.py +++ b/tests/test_client_ws_functional.py @@ -1,7 +1,7 @@ import asyncio import json import sys -from typing import NoReturn, Optional +from typing import List, NoReturn, Optional from unittest import mock import pytest @@ -1232,3 +1232,71 @@ async def test_ws_connect_with_wrong_ssl_type(aiohttp_client: AiohttpClient) -> with pytest.raises(TypeError, match="ssl should be SSLContext, .*"): await session.ws_connect("/", ssl=42) + + +async def test_websocket_connection_not_closed_properly( + aiohttp_client: AiohttpClient, +) -> None: + """Test that closing the connection via __del__ does not raise an exception.""" + + async def handler(request: web.Request) -> NoReturn: + ws = web.WebSocketResponse() + await ws.prepare(request) + await ws.close() + assert False + + app = web.Application() + app.router.add_route("GET", "/", handler) + client = await aiohttp_client(app) + resp = await client.ws_connect("/") + assert resp._conn is not None + # Simulate the connection not being closed properly + # https://github.com/aio-libs/aiohttp/issues/9880 + resp._conn.release() + + # Clean up so the test does not leak + await resp.close() + + +async def test_websocket_connection_cancellation( + aiohttp_client: AiohttpClient, loop: asyncio.AbstractEventLoop +) -> None: + """Test canceling the WebSocket connection task does not raise an exception in __del__.""" + + async def handler(request: web.Request) -> NoReturn: + ws = web.WebSocketResponse() + await ws.prepare(request) + await ws.close() + assert False + + app = web.Application() + app.router.add_route("GET", "/", handler) + + sync_future: "asyncio.Future[List[aiohttp.ClientWebSocketResponse]]" = ( + loop.create_future() + ) + client = await aiohttp_client(app) + + async def websocket_task() -> None: + resp = await client.ws_connect("/") + assert resp is not None # ensure we hold a reference to the response + # The test harness will cleanup the unclosed websocket + # for us, so we need to copy the websockets to ensure + # we can control the cleanup + sync_future.set_result(client._websockets.copy()) + client._websockets.clear() + await asyncio.sleep(0) + + task = loop.create_task(websocket_task()) + websockets = await sync_future + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + websocket = websockets.pop() + # Call the `__del__` methods manually since when it gets gc'd it not reproducible + del websocket._response + + # Cleanup properly + websocket._response = mock.Mock() + await websocket.close()