Unhandled exception for ping task

[greshilov]

🐞 Describe the bug

Websocket handler with enabled autoping option does not handle sudden client disconnection right.

When client calls receive() method, handler internally creates task with ping() call. If the client disconnects without sending close message, task will not be cancelled which leads us to the ConnectionResetError exception. asyncio logs it with Task exception was never retrieved, which is correct, but in this case annoying.

💡 To Reproduce

Consider following server/client example.

# server.py
from aiohttp import web


async def handler(request):
    ws_response = web.WebSocketResponse(heartbeat=2)
    await ws_response.prepare(request)
    await ws_response.receive()
    return ws_response


app = web.Application()
app.add_routes([
    web.get('/ws', handler)
])

web.run_app(app, host='localhost')

# client.py
import asyncio
import aiohttp


async def client_intruder():
    async with aiohttp.ClientSession() as session:
        await session._ws_connect('http://localhost:8080/ws')
        # Look at me! I'm not closing my websocket connection >:P

asyncio.run(client_intruder(), debug=True)

💡 Expected behavior

Exception should be handled. We can't require WSMsgType.CLOSE message from client in all cases, because its disconnection may be caused by network failure.

📋 Logs/tracebacks

Task exception was never retrieved
future: <Task finished name='Task-6' coro=<WebSocketWriter.ping() done, defined at /home/s/python/aiohttp/aiohttp/http_websocket.py:660> exception=ConnectionResetError('Cannot write to closing transport') created at /home/s/python/aiohttp/aiohttp/web_ws.py:136>
source_traceback: Object created at (most recent call last):
  File "server.py", line 17, in <module>
    web.run_app(app, host='localhost', debug=True)
  File "/home/s/python/aiohttp/aiohttp/web.py", line 514, in run_app
    loop.run_until_complete(main_task)
  File "/usr/lib/python3.8/asyncio/base_events.py", line 603, in run_until_complete
    self.run_forever()
  File "/usr/lib/python3.8/asyncio/base_events.py", line 570, in run_forever
    self._run_once()
  File "/usr/lib/python3.8/asyncio/base_events.py", line 1851, in _run_once
    handle._run()
  File "/usr/lib/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/home/s/python/aiohttp/aiohttp/web_ws.py", line 136, in _send_heartbeat
    self._loop.create_task(self._writer.ping())  # type: ignore
Traceback (most recent call last):
  File "/home/s/python/aiohttp/aiohttp/http_websocket.py", line 664, in ping
    await self._send_frame(message, WSMsgType.PING)
  File "/home/s/python/aiohttp/aiohttp/http_websocket.py", line 641, in _send_frame
    self._write(header + message)
  File "/home/s/python/aiohttp/aiohttp/http_websocket.py", line 651, in _write
    raise ConnectionResetError("Cannot write to closing transport")
ConnectionResetError: Cannot write to closing transport

📋 Your version of the Python

$ python --version
Python 3.8.6

📋 Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Name: aiohttp
Version: 4.0.0a1
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: Nikolay Kim
Author-email: [email protected]
License: Apache 2
Location: /home/s/python/aiohttp
Requires: attrs, chardet, multidict, async-timeout, yarl, typing-extensions
Required-by:

$ python -m pip show multidict
Name: multidict
Version: 5.0.0
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /home/s/python/aiohttp/.direnv/python-3.8.6/lib/python3.8/site-packages
Requires: 
Required-by: yarl, aiohttp

$ python -m pip show yarl
Name: yarl
Version: 1.6.1
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: [email protected]
License: Apache 2
Location: /home/s/python/aiohttp/.direnv/python-3.8.6/lib/python3.8/site-packages
Requires: multidict, idna
Required-by: aiohttp

📋 Additional context

Looks like this problem affects 3.7+ versions of aiohttp.

[greshilov]

Possible solution is create method _ping in WebSocketResponse like so:

class WebSocketResponse:
    ...
    async def _ping(self):
        try:
            await self._writer.ping()
        except ConnectionResetError:
            pass

    def _send_heartbeat(self) -> None:
        ...
        self._loop.create_task(self._ping())  # instead of self._writer.ping()

Anyway I can do a PR with a fix :)

[asvetlov]

Hi. There is (not finished yet) #4000 that rewrites WebSockets slightly. I believe it will solve this problem among many others. Stay tuned.

[Programmierus]

Any update on this?

[altendky]

*ping* (it's ok, I know I'm not funny...)

I would like to stop seeing these messages in my logs. I figured instead of just a ping I'd provide some concrete code to try to induce chatter. See https://github.com/aio-libs/aiohttp/pull/7238. It's draft, no doc updates, etc. If it is received well I will do all the pieces of a proper contribution.

Thanks for your time.

[Dreamsorcerer]

I think the previously linked PR looks more promising, as it is attempting to fix the cause of the issue, rather than just hide the error. Not sure why the PR has not been finished off and merged though..

[DOABrownie]

is this issue still present as I still get the error: /usr/lib/python3.11/concurrent/futures/thread.py:59: RuntimeWarning: coroutine 'ClientSession._ws_connect' was never awaited result = self.fn(*self.args, **self.kwargs) RuntimeWarning: Enable tracemalloc to get the object allocation traceback

after I cancel asyncio gathered tasks, it looks like it is coming from: venv/lib64/python3.11/site-packages/aiohttp/client.py line 725