Skip to content

Log WebSocket server handler errors; answer bad handshakes with 400 - #1482

Merged
quinnj merged 6 commits into
masterfrom
claude/ws-server-handler-errors
Oct 22, 2026
Merged

quinnj merged 6 commits into
masterfrom
claude/ws-server-handler-errors

Conversation

@quinnj

@quinnj quinnj commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

WebSockets.listen! serves each connection in a task that nothing waits on. Two failures in that task were silent:

  • Handler errors. When the handler threw, _serve_ws_session! closed the connection with 1011 and rethrew. The rethrown error ended the task and was never reported. A typo in a WebSocket handler showed up only as a client seeing 1011. An error thrown by a check_origin callback was lost the same way.
  • Malformed handshakes. When read_request threw on a malformed handshake, the connection closed without any response.

_serve_ws_conn! now catches what reaches it. A ParseError or ProtocolError from reading the handshake request gets 400 Bad Request. Only those errors get it: an error the handler raises after the 101, even a ProtocolError, is logged, and no HTTP bytes follow the close frame. Errors that come from the peer are not logged: a WebSocketError (the peer closed or broke the protocol), EOF, a closed or TLS-failed connection, a timeout, or a reset or broken pipe. Anything else is logged at error level as websocket server error, with the exception and backtrace. The client still sees 1011 for a handler error, as before.

Verification. New testset in test/http_websocket_server_tests.jl. A handler that throws error("handler bug") makes the client see 1011, and the server logs one websocket server error record carrying that ErrorException. A handler that throws HTTP.ProtocolError after the upgrade is also logged, and the raw client sees the 101, a 1011 close frame and then EOF, with no HTTP response. A handshake with a malformed header line gets 400 Bad Request. On master no record is logged and the reply is empty. Locally, http_websocket_server_tests.jl (73), http_websocket_integration_tests.jl and http_websocket_client_tests.jl pass.

Review. _serve_ws_session! still closes the WebSocket before rethrowing, so the 1011 close frame is unchanged. isok errors (1000, 1001, 1005) are not rethrown, so a normal close or a server shutdown logs nothing. The new catch is inside the existing try/finally, so the connection is still closed and untracked on every path, and close(server) still waits for the task. WebSockets.upgrade inside HTTP.listen! is unaffected: there the HTTP server's own error path logs handler errors.

🤖 Generated with Claude Code

quinnj and others added 3 commits October 22, 2026 02:31
WebSockets.listen! ran each connection in a task that nothing waits on. A
handler that threw closed its connection with 1011 and the error was rethrown
into that task, so it was never reported anywhere; an error from a
check_origin callback was lost the same way. A malformed handshake request
closed the connection without a response. Such errors are now logged as
websocket server error, errors that come from the peer are not, and a
malformed handshake gets 400 Bad Request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.80%. Comparing base (089e78a) to head (9365915).
⚠️ Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1482      +/-   ##
==========================================
+ Coverage   91.67%   91.80%   +0.13%     
==========================================
  Files          31       31              
  Lines       13187    13305     +118     
==========================================
+ Hits        12089    12215     +126     
+ Misses       1098     1090       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…pgrade

A ParseError or ProtocolError raised by the handler after the 101 was taken
for a malformed handshake: the server wrote a 400 response into the upgraded
connection after its close frame and did not log the handler's error. Only an
error from reading the handshake request now gets 400; any later error that
did not come from the peer is logged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts:
#	CHANGELOG.md
#	src/http_websockets.jl
#	test/http_websocket_server_tests.jl
@quinnj
quinnj merged commit 6c49cf8 into master Oct 22, 2026
0 of 8 checks passed
@quinnj
quinnj deleted the claude/ws-server-handler-errors branch October 22, 2026 13:25
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

Successfully merging this pull request may close these issues.

1 participant