fix: add WebSocket readyState guard to prevent send on non-OPEN socket - #827
Merged
Conversation
Closes #826 sendRawMessage() called socket.send() without checking readyState, which throws when the WebSocket is still in CONNECTING state. This could break the reconnection loop entirely, leaving the client permanently disconnected. Changes: - sendRawMessage() now checks readyState === OPEN before sending - sendPing() checks readyState before sending ping messages - Re-subscription loop in connection_ack handler logs per-subscription warnings when messages are dropped
… socket Instead of silently dropping messages and waiting up to 32s for the ping/pong cycle to detect the issue, sendMessage now calls handleConnectionDrop() immediately when sendRawMessage fails.
…e reconnect on re-subscription failure - Remove logging from sendRawMessage so callers provide context-specific messages - Remove redundant readyState guard in sendPing (sendRawMessage already checks) - Trigger handleConnectionDrop() in connection_ack when re-subscriptions fail - Use ConnectionStatus enum instead of raw string in tests
…mments - Switch forEach to for...of with early break when first re-subscription fails (all subsequent sends would also fail on same dead socket) - Wrap connection_ack case body in braces for proper let scoping - Add comments explaining Object.defineProperty readyState overrides and mock-socket message delivery flush in tests
KaarlisCaune
approved these changes
Mar 19, 2026
KaarlisCaune
left a comment
KaarlisCaune
left a comment
Contributor
There was a problem hiding this comment.
Tested by binding to dashboard and manually triggering socket.close() - not really what this PR is fixing, but can confirm reconnection works as expected. The tests seem to do a good job verifying the non-open socket case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #826 (duplicate of #740)
sendRawMessage()checksreadyState === WebSocket.OPENbefore callingsocket.send(), preventing the "Still in CONNECTING state" errorsendMessage()triggers immediate reconnection viahandleConnectionDrop()when a send fails, instead of waiting up to 32s for the ping/pong cyclesendPing()checksreadyStatebefore sending ping messages (pong timeout is still always set as a reconnection safety net)connection_ackhandler logs per-subscription warnings when messages are droppedWithout this fix, the thrown error could break the reconnection loop entirely, leaving the client permanently disconnected.
Test plan