fix(server): don't silently swallow WebSocket message processing errors - #337
fix(server): don't silently swallow WebSocket message processing errors#337klioen wants to merge 1 commit into
Conversation
A single malformed or unexpected message (e.g. a missing field in a Jupyter kernel message) currently bubbles out of _process_message, which terminates the whole receive loop and then marks every ongoing execution as WebSocketError + UnexpectedEndOfExecution. The original exception is only logged as a one-line message without stack trace, so the root cause is effectively swallowed while all in-flight executions get a misleading 'connection lost' error. Isolate per-message processing so that: - a per-message failure logs the full stack trace and a preview of the raw message instead of being silently dropped, - the affected execution is notified with a MessageProcessingError + EndOfExecution so it doesn't hang waiting for results, - the receive loop keeps running for subsequent messages, - only connection-level failures (ConnectionClosedError/WebSocketException) still terminate the loop and cancel all ongoing executions.
|
We require contributors to sign our Contributor License Agreement, and we don't have @klioen on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
|
Closing: this was created as a test. Thanks! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f27836a65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| parent_msg_id = None | ||
| try: | ||
| parent_msg_id = json.loads(message).get( | ||
| "parent_header", {} | ||
| ).get("msg_id") | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
When malformed JSON—or a valid message missing parent_header—is received while an execution is active, this recovery attempt leaves parent_msg_id unset and then continues the receive loop without queuing any terminal marker. If the discarded frame was the execution's final idle status, _wait_for_result() emits keepalives indefinitely; previously the outer finally ended the request with a WebSocket error. Fail the active execution(s), or terminate the receive loop, when the malformed message cannot be associated with a parent.
Useful? React with 👍 / 👎.
1 participant
Fixes #336
Problem
ContextWebSocket._receive_message()processes every WebSocket message with no per-message error isolation:A single malformed or unexpected message (bad JSON, missing
content/headerfield, unexpected structure) raises inside the loop, which:WebSocket received error while receiving messages: <str(e)>— no stack trace, no message content → the real cause is silently swallowed.async forloop →_receive_taskexits → all subsequent kernel messages are never processed and the connection is never re-established.finallyblock which marks every in-flight execution asWebSocketError+UnexpectedEndOfExecution("The connections was lost..."), even though the connection itself is healthy.Fix
Isolate per-message handling in an inner
try/except:ConnectionClosedError/WebSocketException) are re-raised so the outer handler still terminates the loop and cancels all ongoing executions — unchanged behavior for real disconnects.logger.exception), so the failure is no longer silent,MessageProcessingError+EndOfExecutionso that execution ends with a clear error instead of hanging,Testing
python3 -m py_compile template/server/messaging.pypasses.Notes
EndOfExecutionis sent afterMessageProcessingErrorso the client's_wait_for_resultloop terminates (it breaks onEND_OF_EXECUTION).