aiohttp icon indicating copy to clipboard operation
aiohttp copied to clipboard

ClientWebSocket heartbeat should throw exceptions

Open alandtse opened this issue 4 years ago • 10 comments

Long story short

A client websocket session with a heartbeat enabled will never indicate the connection is closed if a program is only receiving information.

This may be related to #2309 (which was server-side, but others appear to be commenting about client-side), where @asvetlov stated:

Workaround (the proper solution actually) is sending ping messages from a client. Unfortunately, browsers don't support WebSocket pings, you have to emulate them by small regular messages.

My understanding is the heartbeat is intended to create the ping message to detect the disconnection. Therefore, when someone is using the heartbeat, an exception should be raised when pong not received.

Expected behaviour

The Timeout exception should be raised upon detecting that _pong_not_received for calling programs to catch.

Actual behaviour

The connection closes silently. The only way to detect it would be to add additional logic to send a ping, which is the purpose of the heartbeat.

Steps to reproduce

  1. Create a receive only websocket connection to any server and enable a heartbeat
async with session.ws_connect('http://example.org/ws', heartbeat=5) as ws:
    async for msg in ws:
        _LOGGER.debug("msg: %s", msg)
  1. Simulate a disconnect by disconnecting internet.
  2. Websocket messages will stop because the connection is closed. However, this is silent.

Your environment

Client OS isn't relevant

alandtse avatar Oct 07 '19 08:10 alandtse

So the solution is that upon exiting the async for msg in ws: loop, you can check the websocket's close reasons including ws.exception(). That will contain the concurrent.futures._base.TimeoutError.

alandtse avatar Oct 08 '19 05:10 alandtse

let me meditate on it

asvetlov avatar Oct 08 '19 12:10 asvetlov

@asvetlov Any update on this?

I came across the same problem as @alandtse, however the solution they provided does not properly address the real problem, as one will only exit the for loop if the connection comes UP again (otherwise it will just hang), which again defies the whole point of having a heartbeat. I do agree however wih the 'Expected Behaviour' they mentioned on their first comment.

ghost avatar Sep 07 '20 11:09 ghost

I have a fix for this issue that works correctly with aiohttp3.6.2 and Python 3.8.5, however aiottp 4.0.0a1 introduces another bug: https://github.com/aio-libs/aiohttp/issues/4526 that supersedes my fix.

Are you still maintaining the stable version or only working on 4.0.*?

ghost avatar Sep 19 '20 18:09 ghost

aiohttp 3.7 is prepared to release; aiohttp 4.0 will probably have reimplemented heartbeats

asvetlov avatar Oct 22 '20 15:10 asvetlov

Where'd we end up here?

jhaenchen avatar Jan 26 '22 20:01 jhaenchen

Have the same issue: heartbeat is on, underlying TCP-connection appeared blocked, _pong_not_received is called but main code still awaits for messages from web-socket

Mirraz avatar Aug 04 '22 18:08 Mirraz

@alandtse @asvetlov @gholt Finally, is it fixed or not ? some intersecting issues.. If yes, in what version ?

socketpair avatar Aug 11 '22 07:08 socketpair

@alandtse seems your explanation is not correct. you said:

Websocket messages will stop because the connection is closed. However, this is silent.

but actually, reading will not be stopped.

socketpair avatar Aug 11 '22 07:08 socketpair

Same issue met here. This is pretty critical as the server maintains resources after the client has vanished, and the heartbeat does not raise an exception about that.

dmoklaf avatar Dec 26 '23 10:12 dmoklaf