Skip to content

Fix the WebSocket close-handshake test race and run it in CI again - #60

Merged
droplister merged 1 commit into
mainfrom
claude/new-session-ta1vaw
Oct 13, 2026
Merged

droplister merged 1 commit into
mainfrom
claude/new-session-ta1vaw

Conversation

@droplister

Copy link
Copy Markdown
Member

tests/websocket-lifecycle.test.ts has two cases that drive a real close handshake against workerd (Miniflare). They were skipped under CI because they failed in containers with code 1006 instead of a clean 1000.

Root cause

The test closed the socket right after open. Probing the same server with two clients, closing immediately:

Client Result (3 runs)
Node built-in WebSocket (undici) 1006, not clean, every time
ws 8.21 1000, clean, every time

Closing 200 ms after open was clean for both. So the server's close handshake is fine. Node's built-in client reports 1006 when it closes while the server's first frame (the presence count / room state) is still arriving.

Fix

  • Wait for the server's first frame before closing, as a real client does.
  • Remove the process.env.CI skip and update the comment, so CI exercises these again.

Testing

  • Passed 5 runs out of 5 in a container, plus once with CI=true.
  • Full suite 1469/1469 (previously 1467 + these 2 failing locally).

🤖 Generated with Claude Code

https://claude.ai/code/session_01LkjKD3QZivr3Qr2cKdjfbz


Generated by Claude Code

The two real-handshake cases closed the socket as soon as it opened.
Node's built-in WebSocket client reports 1006 when it closes while the
server's first frame is still arriving; the ws client gets a clean 1000
against the same workerd server, so the race was the client's. They were
skipped under CI on the theory that containers can't run them.

Close after the first server frame instead, as a real client would, and
drop the CI skip: they now pass repeatedly in a container, with and
without CI=true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LkjKD3QZivr3Qr2cKdjfbz
@droplister
droplister merged commit d326c23 into main Oct 13, 2026
1 check passed
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.

2 participants