Skip to content

fix: prevent One-Way WebSocket error messages for closed connections in React Dev Mode #17204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025
Merged

fix: prevent One-Way WebSocket error messages for closed connections in React Dev Mode #17204

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions site/src/utils/OneWayWebSocket.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ export class OneWayWebSocket<TData = unknown>
implements OneWayWebSocketApi<TData>
{
readonly #socket: WebSocket;
readonly #messageCallbackWrappers = new Map<
readonly #errorListeners = new Set<(e: Event) => void>();
readonly #messageListenerWrappers = new Map<
OneWayEventCallback<TData, "message">,
WebSocketMessageCallback
>();
Expand All @@ -98,7 +99,7 @@ export class OneWayWebSocket<TData = unknown>
} = init;

if (!apiRoute.startsWith("/api/v2/")) {
throw new Error(`API route '${apiRoute}' does not begin with a slash`);
throw new Error(`API route '${apiRoute}' does not begin with '/api/v2/'`);
}

const formattedParams =
Expand All @@ -122,6 +123,10 @@ export class OneWayWebSocket<TData = unknown>
event: TEvent,
callback: OneWayEventCallback<TData, TEvent>,
): void {
if (this.#socket.readyState === WebSocket.CLOSED) {
return;
}

// Not happy about all the type assertions, but there are some nasty
// type contravariance issues if you try to resolve the function types
// properly. This is actually the lesser of two evils
Expand All @@ -130,11 +135,16 @@ export class OneWayWebSocket<TData = unknown>
WebSocketEventType
>;

if (this.#messageCallbackWrappers.has(looseCallback)) {
// WebSockets automatically handle de-duping callbacks, but we have to
// do a separate check for the wrappers
if (this.#messageListenerWrappers.has(looseCallback)) {
return;
}
if (event !== "message") {
this.#socket.addEventListener(event, looseCallback);
if (event === "error") {
this.#errorListeners.add(looseCallback);
}
return;
}

Expand All @@ -161,7 +171,7 @@ export class OneWayWebSocket<TData = unknown>
};

this.#socket.addEventListener(event as "message", wrapped);
this.#messageCallbackWrappers.set(looseCallback, wrapped);
this.#messageListenerWrappers.set(looseCallback, wrapped);
}

removeEventListener<TEvent extends WebSocketEventType>(
Expand All @@ -175,24 +185,37 @@ export class OneWayWebSocket<TData = unknown>

if (event !== "message") {
this.#socket.removeEventListener(event, looseCallback);
if (event === "error") {
this.#errorListeners.delete(looseCallback);
}
return;
}
if (!this.#messageCallbackWrappers.has(looseCallback)) {
if (!this.#messageListenerWrappers.has(looseCallback)) {
return;
}

const wrapper = this.#messageCallbackWrappers.get(looseCallback);
const wrapper = this.#messageListenerWrappers.get(looseCallback);
if (wrapper === undefined) {
throw new Error(
`Cannot unregister callback for event ${event}. This is likely an issue with the browser itself.`,
);
}

this.#socket.removeEventListener(event as "message", wrapper);
this.#messageCallbackWrappers.delete(looseCallback);
this.#messageListenerWrappers.delete(looseCallback);
}

close(closeCode?: number, reason?: string): void {
// Eject all error event listeners, mainly for ergonomics in React dev
// mode. React's StrictMode will create additional connections to ensure
// there aren't any render bugs, but manually closing a connection via a
// cleanup function sometimes causes error events to get dispatched for
// a connection that is no longer wired up to the UI
for (const cb of this.#errorListeners) {
this.#socket.removeEventListener("error", cb);
this.#errorListeners.delete(cb);
}

this.#socket.close(closeCode, reason);
}
}
Loading