fix(websocket): queue close/error events asynchronously when closing during CONNECTING

[maruthang]

This relates to...

Fixes #4741

Rationale

Per the WHATWG WebSocket spec, when close() is invoked on a WebSocket whose readyState is CONNECTING, the implementation must "fail the WebSocket connection", and all resulting event dispatches (error, close) must be delivered via a queued task — not synchronously from the close() call itself.

Prior to this PR, undici dispatched those events synchronously from inside close() via failWebsocketConnection → handler.onSocketClose(). User code observed error and close events before the close() method returned, which is observably different from browser behavior and breaks consumers that set listeners after calling close().

The fix is intentionally scoped to the CONNECTING branch of closeWebSocketConnection. Other callers of failWebsocketConnection (receiver errors, handshake failures, WebSocketStream's abort listener) keep their current synchronous behavior, preserving existing tests such as test/websocket/stream/abort-before-open.js.

Changes

• lib/web/websocket/connection.js: In closeWebSocketConnection, when the socket is still CONNECTING, continue to abort the underlying fetch synchronously and set readyState = CLOSING (so that close() returns with the correct observable state), but defer the onSocketClose() dispatch — which fires error and close — to a queueMicrotask. The microtask guards against re-entry (e.g. a racing socket close or a second close() call) by bailing when readyState is already CLOSED.
• test/websocket/issue-4741.js: New regression test asserting that no error or close event is dispatched synchronously from close() when the socket is in CONNECTING.

Features

N/A

Bug Fixes

• Queue close/error events asynchronously in closeWebSocketConnection when the socket is still CONNECTING (WebSocket events fire synchronously during close() instead of asynchronously #4741).

Breaking Changes and Deprecations

None.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• [S] Benchmarked (optional)
• [S] Documented (no public API surface changed)
• Review ready
• In review
• Merge ready

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.11%. Comparing base (ec60a7c) to head (52c0f6a).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5078   +/-   ##
=======================================
  Coverage   93.10%   93.11%           
=======================================
  Files         110      110           
  Lines       35807    35822   +15     
=======================================
+ Hits        33339    33354   +15     
  Misses       2468     2468           

☔ View full report in Codecov by Sentry.
📢 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.

[KhafraDev]

Please look at prior PRs before opening slop PRs and wasting our time reviewing. Jfc.

#4745 (comment)

[maruthang]

Apologies @KhafraDev — I hadn't seen #4745 or your comment there before opening this, and you're right that a localized microtask in closeWebSocketConnection is the wrong shape for this problem. The spec-aligned fix ("queue a task") belongs one level up in establishWebSocketConnection / closeWebSocketConnection / the message-received path, and once that exists the state-transition guards I added shouldn't be necessary.

Closing this in favor of going back and doing it properly. I'll study the existing work on #4745 first so I'm not duplicating effort or getting in your way. If a fresh PR would be useful at some point, I'll only open one after reading the thread and only if it's clearly a different / complementary direction — otherwise I'll stay out of it.

Sorry for the review time.