fix: robust websocket disconnect, nonblocking usage stats #1655
Conversation
However, when many notebooks are open in marimo edit, some notebooks still initially connect but then disconnect with code 1006 (abnormal disconnection). Frontend still thinks it's connected in these cases ...
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
marimo/_server/sessions.py
Outdated
| self.unsubscribe_consumer() | ||
| self.session_consumer = None | ||
| try: | ||
| self.session_consumer.on_stop() |
There was a problem hiding this comment.
This sometimes fails with errors deep in the guts of websockets. Finally block makes sure the consumer is removed anyway, so users can still work with the notebook by refreshing
| self._on_disconnect( | ||
| e, | ||
| cleanup_fn=lambda: listen_for_disconnect_task.cancel(), | ||
| ) |
There was a problem hiding this comment.
Abnormal disconnects can happen when sending too — and it seems like these can race with the listen_for_disconnect task?
| # Starlette can raise a runtime error if a message is sent | ||
| # when the socket is closed. In case the disconnection | ||
| # error hasn't made its way to listen_for_disconnect, do |
There was a problem hiding this comment.
I am not really sure if this case is needed. I occasionally saw RuntimeErrors raised on send_text saying the socket was already closed
|
|
| @@ -35,6 +35,9 @@ export function useWebSocket(options: UseWebSocketOptions) { | |||
| maxRetries: 10, | |||
| debug: false, | |||
| startClosed: true, | |||
| // long timeout -- the server can become slow when many notebooks | |||
| // are open. | |||
| connectionTimeout: 60_000, | |||
There was a problem hiding this comment.
looks like the default is 4s (just to have it handy here) https://github.com/partykit/partykit/blob/50715a06242732ac828993d2c15d63d30846efb2/packages/partysocket/src/ws.ts#L120
akshayka
changed the title
When multiple notebooks are open in marimo edit, we sometimes get unexpected
WebSocketDisconnects. This change makes the server more robust to those errors — one can at least continue working with notebooks that failed to connect by refreshing — without solving the root problem.Make usage endpoint's CPU measurement nonblocking. This was slowing down the server.
Fixes #1653
Fixes #1622