Skip to content

fix: resolve WebSocket stale closure bug in useRemoteConnection hook#81

Open
Rozerxshashank wants to merge 1 commit intoAOSSIE-Org:mainfrom
Rozerxshashank:fix/websocket-stale-closure
Open

fix: resolve WebSocket stale closure bug in useRemoteConnection hook#81
Rozerxshashank wants to merge 1 commit intoAOSSIE-Org:mainfrom
Rozerxshashank:fix/websocket-stale-closure

Conversation

@Rozerxshashank
Copy link

@Rozerxshashank Rozerxshashank commented Feb 13, 2026

Fixes #80

Problem:
The useRemoteConnection hook stored the WebSocket instance using useState. The useEffect cleanup function captured the initial state value (null) via closure, so ws?.close() on unmount never actually closed the real socket. This caused:

  • Zombie WebSocket connections that stayed open after navigating away
  • Pending reconnect timers firing after the component was gone
  • WebSocket is closed before the connection is established browser warnings during navigation (React Strict Mode double-mounting)
    Fix useRemoteConnection.ts :
  1. Replaced useState<WebSocket> with useRef<WebSocket> so cleanup always accesses the current socket
  2. Added an isMounted flag to prevent reconnection attempts after unmount
  3. Deferred initial socket creation with setTimeout(connect, 0) to avoid creating sockets during React Strict Mode's instant unmount cycle
  4. Nullified event handlers (onopen, onclose, onerror before calling .close() to prevent cascading error events
  5. Stabilized send and sendCombo callbacks with empty dependency arrays since ref identity never changes

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions
  • If applicable, I have made corresponding changes or additions to the documentation
  • If applicable, I have made corresponding changes or additions to tests
  • My changes generate no new warnings or errors
  • I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • I have read the Contribution Guidelines
  • Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

Fixed a WebSocket lifecycle bug in useRemoteConnection.ts by replacing state-based management with ref-based storage, introducing an isMounted guard to prevent post-unmount actions, ensuring sockets close properly, and improving cleanup logic to eliminate zombie connections.

Changes

Cohort / File(s) Summary
WebSocket Lifecycle Management
src/hooks/useRemoteConnection.ts
Converted WebSocket from useState to useRef (wsRef), added isMounted guard to prevent reconnection after unmount, debounced initial connection with 0ms timer, forcibly closes existing sockets before new connections, improved cleanup to safely clear timers and detach handlers, updated send operations to use wsRef.current.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A socket that lingered, long after its day,
Haunting the server in zombie's own way,
But refs hold their truth where state fades to dust,
A mounted guard watches—now closures we trust!
No phantom connections shall roam anymore, hop!

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: fixing a WebSocket stale closure bug in the useRemoteConnection hook.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from issue #80: replaced useState with useRef, added isMounted flag, prevented post-unmount reconnections, stabilized callbacks, and closed sockets properly.
Out of Scope Changes check ✅ Passed All changes are directly scoped to resolving the WebSocket stale closure bug described in issue #80; no unrelated modifications were introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: WebSocket stale closure causes failed cleanup and zombie connections on reconnect

1 participant