Skip to content

Shared socket torn down before listeners attach, throws "Cannot set properties of undefined (setting 'onmessage')" #284

Description

@sgatanadev

TL;DR

share: true - shared socket can be deleted between creation and attachSharedListeners, throwing Cannot set properties of undefined (setting 'onmessage')

Version: 4.13.0

Summary

When two (or more) hook instances subscribe to the same URL with share: true, the shared WebSocket can be created and then immediately deleted from sharedWebSockets by another instance's unmount cleanup before attachSharedListeners runs against it, because the new subscriber isn't registered in the subscriber map until after listener attachment. This throws:

Cannot set properties of undefined (setting 'onmessage')
Cannot set properties of undefined (setting 'onopen')
Cannot set properties of undefined (setting 'onclose')
Cannot set properties of undefined (setting 'onerror')

Where it happens

createOrJoinSocket in create-or-join.ts (compiled: dist/lib/create-or-join.js:45-65):

if (sharedWebSockets[url] === undefined) {
  sharedWebSockets[url] = new WebSocket(url, optionsRef.current.protocols);
  webSocketRef.current = sharedWebSockets[url];
  setReadyState(ReadyState.CONNECTING);              // (A) flushSync - can run another effect's cleanup here
  clearSocketIoPingInterval = attachSharedListeners(   // (B) re-reads sharedWebSockets[url] - may now be undefined
    sharedWebSockets[url], url, optionsRef, sendMessage
  );
}
...
addSubscriber(url, subscriber);   // (C) only happens AFTER (B)

setReadyState at (A) is protectedSetReadyState from use-websocket.ts, which wraps the update in flushSync. In React 18, flushSync synchronously drains any other pending passive effects before it returns. If a different hook instance sharing the same url has an unmount cleanup queued at that instant - a real unmount, or React StrictMode's synchronous mount -> cleanup -> remount dev cycle - that cleanup executes inside this flushSync call, i.e. in the gap between (A) and (B).

That cleanup is cleanSubscribers (create-or-join.ts, dist/lib/create-or-join.js:10-33):

if (!hasSubscribers(url)) {
  ...
  socketLike.close();
  delete sharedWebSockets[url];
}

Because the new subscriber isn't added until (C) - after listener attachment - hasSubscribers(url) is still false at the moment the interleaved cleanup runs, even though a fresh socket was just assigned one line earlier at (A). The cleanup tears it down. Execution returns from flushSync, and (B) re-reads sharedWebSockets[url], now undefined, and hands it to attachSharedListeners.

bindMessageHandler/bindOpenHandler/bindCloseHandler/bindErrorHandler in attach-shared-listeners.ts (dist/lib/attach-shared-listeners.js:20-118) all do webSocketInstance.onXxx = ... unconditionally - that's the throw site.

The await getUrl(url, optionsCache) in the useWebSocket effect (use-websocket.ts, dist/lib/use-websocket.js:126-160) is what allows two subscribers to the same URL to have overlapping in-flight mount sequences in the first place - it's the precondition that opens the door, not where the throw occurs.

Repro sketch

function Consumer() {
  useWebSocket(SAME_URL, { share: true });
  return null;
}

function App({ showBoth }: { showBoth: boolean }) {
  return (
    <>
      <Consumer />
      {showBoth && <Consumer />}
    </>
  );
}

Rapidly toggling showBoth (or running under <StrictMode> in dev, which performs a synchronous mount -> cleanup -> remount for every effect) reproduces the interleaving needed to trigger the race. It's timing-dependent, not deterministic on every toggle.

Suggested fix (other suggestions are welcome)

Register the subscriber before any code path that can trigger a synchronous re-entrant flush, so hasSubscribers(url) is already true if a concurrent cleanup runs mid-creation:

if (sharedWebSockets[url] === undefined) {
  sharedWebSockets[url] = new WebSocket(url, optionsRef.current.protocols);
  webSocketRef.current = sharedWebSockets[url];

  let subscriber = { setLastMessage, setReadyState, optionsRef, reconnectCount, lastMessageTime, reconnect: startRef };
  addSubscriber(url, subscriber);           // moved up, before setReadyState/attachSharedListeners

  setReadyState(ReadyState.CONNECTING);
  clearSocketIoPingInterval = attachSharedListeners(sharedWebSockets[url], url, optionsRef, sendMessage);
} else {
  webSocketRef.current = sharedWebSockets[url];
  setReadyState(sharedWebSockets[url].readyState);
  var subscriber = { ... };
  addSubscriber(url, subscriber);
}
return cleanSubscribers(url, subscriber, optionsRef, setReadyState, clearSocketIoPingInterval);

As defense in depth, it'd also be worth:

  1. Guarding attachSharedListeners (or its bindXxx helpers) against a null/undefined webSocketInstance rather than assuming it's always live.
  2. Having cleanSubscribers capture and compare against the specific socket instance it was handed, so it only closes/deletes the entry in sharedWebSockets[url] if it's still the same instance - protecting against the same class of race even if reordering above misses an edge case.

Why this matters for us

We currently drop these two error message patterns (Cannot set propert(?:y|ies) of undefined... / can't access property "on(message|open|close|error)"...) from undefined/null, scoped to sources matching react-use-websocket|attach-shared-listeners|create-or-join, before they reach our error monitoring (PostHog/Sentry) - the errors are harmless but were spiking our monitoring volume. We'd like to remove that filter once this is fixed upstream.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions