Skip to content

Post-connect WebSocket error leaks native Event as an unhandled rejection #2005

Description

@anaibol

SDK version

livekit-client@2.20.0

Summary

A post-connect WebSocket error event can escape as an unhandled native Event (typically rendered as [object Event]) instead of a ConnectionError. If the browser does not emit a timely close event, the connected signal client also does not route the failure through its existing close/recovery path.

Source-level reproduction

This reproduces deterministically with a fake WebSocket and does not require a server:

  1. Construct WebSocketStream and emit open.
  2. Read from connection.readable.getReader().
  3. Emit new Event('error') from the fake socket after the connection is open.
  4. Observe that reader.read() rejects with the exact native Event, so String(reason) === '[object Event]'.
  5. In SignalClient, handleSignalConnected() starts startReadingLoop() without observing the returned promise, so the reader rejection becomes unhandled.
  6. If no close event follows within 250 ms, WebSocketStream.closed rejects, but the connected-state branch of SignalClient does not call handleOnClose().

The relevant paths are:

  • src/api/WebSocketStream.ts: the post-open ws.onerror passes the native event to controller.error.
  • src/api/SignalClient.ts: handleSignalConnected does not observe startReadingLoop rejection.
  • src/api/SignalClient.ts: the ws.closed.catch handler only handles connection establishment.

Expected behavior

  • Post-connect socket errors reject the readable stream with ConnectionError.websocket(...), not a native event.
  • The signal reader promise is always observed.
  • Recovery runs exactly once:
    • error followed by abnormal close: the existing closed.then close path owns recovery;
    • error without a timely close: closed.catch routes the connected client through handleOnClose;
    • normal close remains unchanged.

Proposed source fix

// WebSocketStream readable start
ws.onerror = () =>
  controller.error(
    ConnectionError.websocket('Encountered websocket error after connection establishment'),
  );

// SignalClient connect: scope every callback to the socket that created it
const ws = new WebSocketStream<ArrayBuffer>(rtcUrl);
this.ws = ws;
ws.closed.catch((reason) => {
  if (this.ws !== ws) return;
  if (this.isEstablishingConnection) {
    reject(/* existing connection error */);
  } else if (this.state === SignalConnectionState.CONNECTED) {
    const closeReason = reason instanceof Error ? reason.message : 'Unexpected WS error';
    void this.handleOnClose(closeReason, ws);
  }
});

// SignalClient.handleSignalConnected
void this.startReadingLoop(connection.readable.getReader(), firstMessage).catch((reason) => {
  this.handleWSError(reason);
});

// handleOnClose should synchronously claim recovery per socket (for example,
// with a WeakSet), then re-check the expected socket after acquiring the close
// lock so a late failure can never close or callback for a replacement socket.

I verified this shape with fake-socket regressions for error-plus-abnormal-close, error-without-close, normal close, a late failure from a replaced socket, and concurrent recovery claims. Each active failure invoked the close callback once, the stale failure invoked none, and no reader rejection was unhandled.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions