Fix websocket disconnect handling to avoid server shutdown#1636
Fix websocket disconnect handling to avoid server shutdown#1636simonrosenberg wants to merge 1 commit intomainfrom
Conversation
Modified websocket handlers to: - Handle ConnectionError gracefully (connection-related, safe to suppress) - Keep re-raising RuntimeError to surface actual bugs - Use explicit except clause for ConnectionError (cleaner than isinstance) Changes: - events_socket: Handle ConnectionError gracefully, re-raise RuntimeError - bash_events_socket: Same handling - Cleanup (unsubscription) still happens in the finally block Fixes #1633 (partial - websocket disconnect fix) Co-authored-by: openhands <openhands@all-hands.dev>
49c73bd to
050ceca
Compare
|
[Automatic Post]: This PR seems to be currently waiting for review. @tofarr, could you please take a look when you have a chance? |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @simonrosenberg, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
[Automatic Post]: This PR seems to be currently waiting for review. @tofarr, could you please take a look when you have a chance? |
3 similar comments
|
[Automatic Post]: This PR seems to be currently waiting for review. @tofarr, could you please take a look when you have a chance? |
|
[Automatic Post]: This PR seems to be currently waiting for review. @tofarr, could you please take a look when you have a chance? |
|
[Automatic Post]: This PR seems to be currently waiting for review. @tofarr, could you please take a look when you have a chance? |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @simonrosenberg, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
Summary
This PR fixes the secondary issue from #1633: websocket disconnect handling that could potentially cause server instability.
Problem
When websocket connections encountered
ConnectionError, these exceptions were being re-raised along withRuntimeError, which could potentially cause server instability.Solution
Modified websocket handlers to:
ConnectionErrorgracefully (connection-related errors are safe to suppress)RuntimeErrorto surface actual bugs (as suggested in code review)except ConnectionErrorclause instead ofisinstancecheck (cleaner)Changes
sockets.py: Modifiedevents_socketandbash_events_socketto handleConnectionErrorgracefully while still re-raisingRuntimeErrortest_event_router_websocket.py: Updated test to verifyRuntimeErroris still re-raisedChecklist
Fixes #1633 (partial - websocket disconnect fix)
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:f2091f2-pythonRun
All tags pushed for this build
About Multi-Architecture Support
f2091f2-python) is a multi-arch manifest supporting both amd64 and arm64f2091f2-python-amd64) are also available if needed