fix: prevent unsubscribe errors by checking WebSocket state before sending #23

jantokic · 2025-11-29T01:11:34Z

Problem

When clients disconnect, unsubscribe() attempts to send messages even when the WebSocket is already closing, causing "Invalid request body" errors that pollute logs.

Solution

Check readyState !== WebSocket.OPEN before attempting unsubscribe (silently return if not ready)
Wrap ws.send() in try-catch to handle synchronous errors
Only log unsubscribe errors when connection is still OPEN
Remove noisy console.log("unsubscribing", ...) debug log
Handle "Invalid request body" error messages gracefully in onMessage

Testing

Tested unsubscribe during disconnection - no errors appear
Verified actual errors still logged when connection is open
No breaking changes

Related Issues

#21

Note

Low Risk
Small, localized changes to client-side WebSocket error handling and logging; main risk is masking an unexpected server error string match or altering when sockets are closed on send failures.

Overview
Reduces noisy WebSocket error logging during disconnect/teardown by ignoring server messages whose JSON message contains "invalid request body".

Hardens subscribe()/unsubscribe() by returning early when the socket is not OPEN, wrapping ws.send() in try/catch to handle synchronous send failures, and only logging/closing the socket on callback errors when the connection is still OPEN (also removing the unsubscribing debug log).

^{Written by Cursor Bugbot for commit ac1f5fd. This will update automatically on new commits. Configure here.}

Copilot

Pull request overview

This PR addresses WebSocket unsubscribe errors that occur when clients disconnect by preventing unsubscribe operations on non-open connections and filtering specific error messages.

Silently return from unsubscribe() when WebSocket is not OPEN instead of logging a warning
Add try-catch around ws.send() and only log errors when connection is still OPEN
Filter "invalid request body" and "unsubscribe" error messages in onMessage to reduce log noise

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-29T01:13:31Z

src/client.ts

+                    }
+                }
+            });
+        } catch (error) {


Empty catch block silently swallows all errors. This could hide legitimate issues like JSON serialization errors or programming mistakes. Consider at least logging unexpected errors or being more specific about which errors to ignore.

Suggested change

} catch (error) {

} catch (error) {

console.error("unsubscribe exception", error);

Copilot · 2025-11-29T01:13:32Z

src/client.ts

+                } catch {
+                    // Not JSON, ignore silently
+                }
            }


The error handling logic removes the original error log when the WebSocket is not OPEN (line 168 removed the unconditional console.log("onMessage error", { event })), but doesn't restore it for legitimate errors. This means valid error messages that don't match the filter criteria will be silently ignored. Consider adding back an error log for cases that don't match the suppression filters.

Suggested change

} catch {

// Not JSON, ignore silently

}

}

// If we reach here, it's a parsed message that isn't suppressed; log it

console.error("onMessage error (unhandled message):", event.data);

} catch {

// Not JSON, log the raw event data for visibility

console.error("onMessage error (non-JSON message):", event.data);

}

Copilot · 2025-11-29T01:13:32Z

src/client.ts

+                    const parsed = JSON.parse(event.data);
+                    if (parsed.message && typeof parsed.message === "string") {
+                        const errorMsg = parsed.message.toLowerCase();
+                        if (errorMsg.includes("invalid request body") || errorMsg.includes("unsubscribe")) {


Checking for "unsubscribe" in error messages is too broad and could suppress legitimate error messages that happen to contain this word. For example, an error like "Failed to unsubscribe from topic X" would be silently ignored. Consider being more specific about which error messages to filter, or only filter "invalid request body" errors.

Suggested change

if (errorMsg.includes("invalid request body") || errorMsg.includes("unsubscribe")) {

if (errorMsg.includes("invalid request body")) {

src/client.ts

…nding

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-29T01:22:32Z

src/client.ts

+        ws.send(JSON.stringify({ action: "subscribe", ...msg }), (err?: Error) => {
            if (err) {
                console.error("subscribe error", err);
-                this.ws.close();
+                ws.close();
            }
        });


The subscribe method should also wrap ws.send() in a try-catch block for consistency with the unsubscribe method. If ws.send() can throw synchronous errors during unsubscribe (which is why a try-catch was added), the same could happen during subscribe operations.

Copilot · 2025-11-29T01:22:32Z

src/client.ts

+            });
+        } catch (error) {
+            if (ws.readyState === WebSocket.OPEN) {
+                console.error("unsubscribe exception", error);


When a synchronous error is caught from ws.send() (line 227), the error is logged but the WebSocket is not closed. This is inconsistent with the callback error handler on line 223, which closes the connection after logging the error. Consider adding ws.close(); after line 229 to maintain consistent error handling behavior.

Suggested change

console.error("unsubscribe exception", error);

console.error("unsubscribe exception", error);

ws.close();

src/client.ts

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-29T01:42:34Z

src/client.ts

+        const ws = this.ws;
+        try {
+            ws.send(JSON.stringify({ action: "subscribe", ...msg }), (err?: Error) => {
+                if (err) {
+                    if (ws.readyState === WebSocket.OPEN) {
+                        console.error("subscribe error", err);
+                        ws.close();
+                    }
+                }
+            });
+        } catch (error) {
+            if (ws.readyState === WebSocket.OPEN) {
+                console.error("subscribe exception", error);
+                ws.close();
            }
-        });
+        }


Race condition: The local ws variable captures this.ws at the time of the method call, but this.ws can be reassigned during reconnection (in the connect() method). The asynchronous callback and catch block check ws.readyState, which refers to the old WebSocket instance, not the current one. This could lead to incorrect behavior if reconnection happens between the send() call and the callback execution.

Consider checking this.ws.readyState instead of ws.readyState, or ensure the callback/error handler operates on the correct WebSocket instance.

Copilot · 2025-11-29T01:42:35Z

src/client.ts

+        const ws = this.ws;
+        try {
+            ws.send(JSON.stringify({ action: "unsubscribe", ...msg }), (err?: Error) => {
+                if (err) {
+                    if (ws.readyState === WebSocket.OPEN) {
+                        console.error("unsubscribe error", err);
+                        ws.close();
+                    }
+                }
+            });
+        } catch (error) {
+            if (ws.readyState === WebSocket.OPEN) {
+                console.error("unsubscribe exception", error);
+                ws.close();
            }
-        });
+        }


Race condition: The local ws variable captures this.ws at the time of the method call, but this.ws can be reassigned during reconnection (in the connect() method). The asynchronous callback and catch block check ws.readyState, which refers to the old WebSocket instance, not the current one. This could lead to incorrect behavior if reconnection happens between the send() call and the callback execution.

Consider checking this.ws.readyState instead of ws.readyState, or ensure the callback/error handler operates on the correct WebSocket instance.

Copilot · 2025-11-29T01:42:35Z

src/client.ts

+        const ws = this.ws;
+        try {
+            ws.send(JSON.stringify({ action: "subscribe", ...msg }), (err?: Error) => {
+                if (err) {
+                    if (ws.readyState === WebSocket.OPEN) {
+                        console.error("subscribe error", err);
+                        ws.close();
+                    }
+                }
+            });
+        } catch (error) {
+            if (ws.readyState === WebSocket.OPEN) {
+                console.error("subscribe exception", error);
+                ws.close();
            }
-        });
+        }


[nitpick] Inconsistent error handling: The ping() method at line 151 doesn't use the same try-catch pattern or state-aware error handling that was added to subscribe() and unsubscribe(). For consistency and to prevent similar errors during disconnection, consider applying the same error handling pattern to ping().

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-06T12:41:55Z

src/client.ts

+                        if (errorMsg.includes("invalid request body")) {
+                            return;
+                        }
+                    }


Silently swallowing all "invalid request body" server errors

Medium Severity

The onMessage handler silently suppresses all server responses containing "invalid request body", not just ones caused by the disconnect race condition. If a genuinely malformed subscribe or unsubscribe request is sent (e.g., wrong fields), the server's error response is silently dropped with no logging, making it very difficult to debug subscription failures. The readyState guards already added to subscribe/unsubscribe prevent the race condition; this broad suppression in onMessage masks legitimate API errors.

jantokic requested a review from a team as a code owner November 29, 2025 01:11

Copilot AI review requested due to automatic review settings November 29, 2025 01:11

Copilot started reviewing on behalf of jantokic November 29, 2025 01:11 View session

Copilot finished reviewing on behalf of jantokic November 29, 2025 01:13

Copilot AI reviewed Nov 29, 2025

View reviewed changes

jantokic force-pushed the fix/graceful-error-handling branch from aa76492 to 28f99ad Compare November 29, 2025 01:14

cursor bot reviewed Nov 29, 2025

View reviewed changes

src/client.ts Outdated Show resolved Hide resolved

fix: prevent unsubscribe errors by checking WebSocket state before se…

6352798

…nding

Copilot AI review requested due to automatic review settings November 29, 2025 01:19

jantokic force-pushed the fix/graceful-error-handling branch from 28f99ad to 6352798 Compare November 29, 2025 01:19

Copilot started reviewing on behalf of jantokic November 29, 2025 01:19 View session

Copilot finished reviewing on behalf of jantokic November 29, 2025 01:22

Copilot AI reviewed Nov 29, 2025

View reviewed changes

fix: add try-catch to subscribe and close ws in unsubscribe catch

3def1f3

cursor bot reviewed Nov 29, 2025

View reviewed changes

src/client.ts Show resolved Hide resolved

fix: check readyState in subscribe error callback for consistency

61de766

Copilot AI review requested due to automatic review settings November 29, 2025 01:40

Copilot started reviewing on behalf of jantokic November 29, 2025 01:40 View session

Copilot finished reviewing on behalf of jantokic November 29, 2025 01:41

Copilot AI reviewed Nov 29, 2025

View reviewed changes

Merge branch 'main' into fix/graceful-error-handling

ac1f5fd

cursor bot reviewed Feb 6, 2026

View reviewed changes

	} catch (error) {
	} catch (error) {
	console.error("unsubscribe exception", error);

-                } catch {
-                    // Not JSON, ignore silently
-                }
-            }
+                    // If we reach here, it's a parsed message that isn't suppressed; log it
+                    console.error("onMessage error (unhandled message):", event.data);
+                } catch {
+                    // Not JSON, log the raw event data for visibility
+                    console.error("onMessage error (non-JSON message):", event.data);
+                }

	if (errorMsg.includes("invalid request body") \|\| errorMsg.includes("unsubscribe")) {
	if (errorMsg.includes("invalid request body")) {

	console.error("unsubscribe exception", error);
	console.error("unsubscribe exception", error);
	ws.close();

fix: prevent unsubscribe errors by checking WebSocket state before sending #23

Are you sure you want to change the base?

fix: prevent unsubscribe errors by checking WebSocket state before sending #23

Uh oh!

Conversation

jantokic commented Nov 29, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Testing

Related Issues

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 29, 2025

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 6, 2026

Choose a reason for hiding this comment

Silently swallowing all "invalid request body" server errors

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jantokic commented Nov 29, 2025 •

edited by cursor bot

Loading