Skip to content

Commit 8e46916

Browse files
bghgaryCopilot
andauthored
Fix WebSocket polyfill throwing on close()/send() when CLOSING or CLOSED (#160)
[Created by Copilot on behalf of @bghgary] ## Problem The WebSocket polyfill throws JavaScript errors in two places where the [WHATWG WebSocket specification](https://websockets.spec.whatwg.org/) requires different behavior: ### `close()` throws when already closing/closed `WebSocket::Close` throws `Error: Close has already been called.` when invoked on a socket whose `readyState` is already `CLOSING` or `CLOSED`. Per the spec, `close()` in those states must be a silent no-op. ### `send()` throws when closing/closed `WebSocket::Send` throws `Error: Websocket readyState is not open.` on any non-OPEN state. Per the spec, `send()` should only throw `InvalidStateError` when `readyState` is `CONNECTING`. When `CLOSING` or `CLOSED`, the data is silently discarded (the spec also bumps `bufferedAmount`, which this polyfill does not track). Because the throws are synchronous from a JS call, a delayed or racing call from an async callback surfaces as an **uncaught** exception that terminates the JS runtime. ## Repro path Observed on the macOS_Xcode164_Sanitizers leg of #148. Sequence (from the existing multi-WebSocket test in `Tests/UnitTests/Scripts/tests.ts`): 1. Test calls `ws.close()` on an open socket. 2. The native side transitions `readyState` -> `Closing` and issues the close. 3. Before the `onclose` callback marshals to the JS thread, another path (a pending `onmessage` handler, or a `catch` block on a failed `send`) calls `ws.close()` again. 4. The second `close()` finds `readyState == Closing` and **throws**, producing: ``` [Uncaught Error] close@[native code] @app:///Scripts/tests.js:27799:22 ``` The `send()` path is symmetric: a send scheduled between `close()` being called and the `onclose` callback firing would throw and escape uncaught. ## Fix ### `close()` Replace the `throw` with an early `return`. No other state is touched, so the first `close()` still drives the transition to `CLOSED` via the normal callback path. ### `send()` Split the single non-OPEN throw into two cases: throw on `CONNECTING`, return silently on `CLOSING`/`CLOSED`. ## Scope - Pure behavioral fix in the WebSocket polyfill. No changes outside `Polyfills/WebSocket/Source/WebSocket.cpp`. - No test changes — existing WebSocket tests already exercise these paths and will stop intermittently failing once this lands. - `bufferedAmount` tracking during the CLOSING/CLOSED send path is still not implemented; out of scope for this fix. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 53f8411 commit 8e46916

1 file changed

Lines changed: 13 additions & 3 deletions

File tree

Polyfills/WebSocket/Source/WebSocket.cpp

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,31 @@ namespace Babylon::Polyfills::Internal
5151
m_cancellationSource->cancel();
5252
}
5353

54-
void WebSocket::Close(const Napi::CallbackInfo& info)
54+
void WebSocket::Close(const Napi::CallbackInfo&)
5555
{
56+
// Per the WHATWG WebSocket spec, calling close() on a socket that is
57+
// already CLOSING or CLOSED is a no-op.
5658
if (m_readyState == ReadyState::Closed || m_readyState == ReadyState::Closing)
5759
{
58-
throw Napi::Error::New(info.Env(), "Close has already been called.");
60+
return;
5961
}
6062
m_readyState = ReadyState::Closing;
6163
m_webSocket.Close();
6264
}
6365

6466
void WebSocket::Send(const Napi::CallbackInfo& info)
6567
{
68+
// Per the WHATWG WebSocket spec, send() throws InvalidStateError only
69+
// when readyState is CONNECTING. When CLOSING or CLOSED, the data is
70+
// silently discarded (the spec still bumps bufferedAmount, which this
71+
// polyfill does not track).
72+
if (m_readyState == ReadyState::Connecting)
73+
{
74+
throw Napi::Error::New(info.Env(), "WebSocket is still in CONNECTING state.");
75+
}
6676
if (m_readyState != ReadyState::Open)
6777
{
68-
throw Napi::Error::New(info.Env(), "Websocket readyState is not open.");
78+
return;
6979
}
7080
std::string message = info[0].As<Napi::String>();
7181
m_webSocket.Send(message);

0 commit comments

Comments
 (0)