fix: resolve stale closure and peer connection cleanup in P2P reconnection#16
Conversation
…ction - Remove stale 'status' variable checks in attemptP2PReconnection that always evaluated to captured initial value - Add explicit cleanup of previous peer connections before creating new ones - Ensure relay fallback logic depends on data channel state, not stale closure variables - Fixes potential logic errors during reconnection attempts and prevents peer connection leaks
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
serotine | b17aa1b | Commit Preview URL Branch Preview URL |
May 22 2026, 10:09 AM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b17aa1b874
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (peerRef.current) { | ||
| peerRef.current.close(); |
There was a problem hiding this comment.
Calling peerRef.current.close() here can trigger the previous data channel’s onclose handler, which calls scheduleReconnection(). Because this happens while a new reconnection attempt is already in progress, a second reconnect timer can be queued and later tear down the fresh connection (or start overlapping attempts), causing reconnect thrash/flapping in unstable networks. This was introduced by the new pre-close step; suppress old handlers (or clear/guard reconnect scheduling) before closing the previous peer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the useP2PChat hook’s P2P reconnection flow to avoid stale React state in reconnection checks, explicitly close prior peer connections between attempts, and make relay fallback decisions based on actual DataChannel state.
Changes:
- Close any existing
RTCPeerConnectionbefore starting a new reconnection attempt. - Remove stale
status-closure gating so relay fallback always executes when reconnection doesn’t succeed. - Simplify reconnection timeout/catch fallback paths to consistently enter relay mode and reschedule attempts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Clean up previous peer connection if it exists | ||
| if (peerRef.current) { | ||
| peerRef.current.close(); | ||
| } |
| setTimeout(async () => { | ||
| clearInterval(answerPoll); | ||
| if (mounted && dc.readyState !== 'open') { | ||
| pc.close(); | ||
| if (status === 'connecting') { | ||
| setStatus('relay'); | ||
| pollBackoffRef.current = 3000; | ||
| setupRelayPolling(); | ||
| await pollRelayMessages(); | ||
| scheduleReconnection(); | ||
| } | ||
| setStatus('relay'); | ||
| pollBackoffRef.current = 3000; | ||
| setupRelayPolling(); | ||
| await pollRelayMessages(); | ||
| scheduleReconnection(); |
Summary
Fixes a critical stale closure bug in the P2P reconnection logic that could cause logic errors and prevent proper fallback to relay mode.
Changes
attemptP2PReconnectionfunction was checking a stalestatusvariable captured from the effect's initial render. Since effects don't re-run when state changes, this variable always held its initial 'connecting' value, making the checks ineffective and misleading.Impact
Testing
Generated by Claude Code