Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Websocket state handling #19

Open
tbronson opened this issue Sep 16, 2024 · 3 comments
Open

Websocket state handling #19

tbronson opened this issue Sep 16, 2024 · 3 comments

Comments

@tbronson
Copy link

Handling of detecting when the websockets close is bugged at the moment, sessions are never cleaned up because of it.

Here are a couple patches that fix the issue:

handler.py

...
async def _receive_loop(self) -> None:
        while True:
            try:
                ws_msg = await self._socket.receive()
                self._socket._raise_on_disconnect(ws_msg)
            except WebSocketDisconnect as e:
                log.info(
                    "WebSocket connection closed: code=%s, reason=%r", e.code, e.reason
                )
                self.application.client_lost(self.connection)
...

receive does not raise when the socket is closed normally, but starlette has a private helper function to do so

...
async def send_text(self, text: str) -> None:
        try:
            await self._socket.send_text(text)
        except WebSocketDisconnect as e:
            self.on_close(e.code, e.reason)

    async def send_bytes(self, bytestream: bytes) -> None:
        try:
            await self._socket.send_bytes(bytestream)
        except WebSocketDisconnect as e:
            self.on_close(e.code, e.reason)
...

send_text and send_bytes do raise, they just needed to be put in a try block

@pmeier
Copy link
Collaborator

pmeier commented Sep 16, 2024

Thanks for the report @tbronson. Could you when exactly this happens? If a client drops the connection without properly closing it?

@tbronson
Copy link
Author

Yes, specifically I found this while testing Panel 1.5.0rc2 on Windows 10.

Launch any panel app with 'admin=True', open the admin panel in a tab, then close the tab and check server console. It will be spammed with unhandled write exceptions to closed websocket (from periodic_callbacks) and the sessions will never be cleaned up either.

@tbronson
Copy link
Author

I should clarify:

The websocket.disconnect message does come in, it's just not handled completely, hence the _raise_on_disconnect call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants