fix: WebRTC auto-reconnect on tab resume + Go Available reconnects first#341
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughEnhanced connection resilience in the Telnyx context layer by adding automatic reconnection attempts when transitioning status to available without registration, implementing visibility-based reconnect triggering, and updating cooldown bypass logic to accommodate specific reconnect reasons. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Header
participant TelnyxCtx
participant Recovery
rect rgba(100, 150, 200, 0.5)
Note over Header,Recovery: Flow 1: Status Change to Available (Unregistered)
User->>Header: Click set status to available
Header->>Header: Check isRegisteredRef
alt isRegistered = false
Header->>TelnyxCtx: Call reconnect()
Header->>Header: Start polling (300ms intervals, max 12s)
loop Poll until registered or timeout
Header->>Header: Check isRegisteredRef
alt Registration successful
Header->>User: Status set to available
end
end
alt Timeout
Header->>User: Show error: "Use Agent Diagnostic → Reconnect"
end
else isRegistered = true
Header->>User: Status set to available
end
end
rect rgba(150, 200, 100, 0.5)
Note over Header,Recovery: Flow 2: Tab Visibility Change
User->>Document: Tab becomes visible
Header->>Header: Listen to visibility change
alt connectionStatus = 'disconnected' or 'error'
Header->>TelnyxCtx: Call reconnect() (bypass cooldown)
Header->>Recovery: Schedule recovery after 500ms
else connectionStatus = 'connected'
Header->>Recovery: Schedule recovery after 1500ms (legacy behavior)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: f3a94da477
ℹ️ 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".
| isRegisteredRef.current = isRegistered | ||
| }, [isRegistered]) | ||
|
|
||
| const waitForRegistration = useCallback(async (timeoutMs = 12000) => { |
There was a problem hiding this comment.
waitForRegistration caps the Go Available recovery wait at 12 seconds, but reconnect attempts in triggerManualReconnect use exponential delays (3s/6s/12s… up to 30s) before init() even runs, and init() itself can take up to the registration timeout. In those common failure-recovery paths, this function returns false before the reconnect attempt can realistically complete, so agents get a hard “cannot go available” error even though the phone may recover shortly after.
Useful? React with 👍 / 👎.
| showPresenceToast('Phone reconnecting…', 'warning') | ||
| reconnect() | ||
|
|
||
| const recovered = await waitForRegistration() |
There was a problem hiding this comment.
This handler now awaits registration before applying the status change, but there is no cancellation/version guard for in-flight requests. If an agent clicks “Go Available” while disconnected and then chooses another status before registration completes, the delayed first call continues and can overwrite the later selection by posting available after the second change has already succeeded.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/layout/header.tsx`:
- Around line 93-103: handleStatusChange triggers reconnect() when moving to
'available', but the reconnect implementation always uses the literal
'manual-button' reason so the 'go-available' bypass branch is never reached;
update the reconnect signature in the UseTelnyxReturn type to accept an optional
reason (e.g., reconnect: (reason?: string) => void), change the hook's reconnect
callback (where manualReconnectFnRef is invoked) to accept a reason parameter
with a 'manual-button' default, and call reconnect('go-available') from
handleStatusChange so the 'go-available' reason is actually passed through and
honored by the bypass logic.
- Around line 160-167: The useEffect in header.tsx that watches connectionStatus
and calls reconnect() duplicates the hook's visibility-based reconnect path;
remove this entire effect (the useEffect that checks document.visibilityState,
connectionStatus and schedules reconnect with setTimeout) and rely on the hook's
handleVisibilityChange / triggerManualReconnect and its runLifecycleRecovery
timing instead (symbols: the useEffect block in header.tsx, connectionStatus,
reconnect; hook symbols: handleVisibilityChange, triggerManualReconnect,
runLifecycleRecovery in use-telnyx). Ensure no other code depends on that
setTimeout retry before removing.
In `@src/hooks/use-telnyx.ts`:
- Around line 885-892: When visibility resumes and connectionStatusRef.current
is 'disconnected' or 'error', avoid invoking both
triggerManualReconnect('visibility-resume') and
runLifecycleRecovery('visibilitychange', 500) to prevent a possible
double-reconnect; change the branch in the visibility handler so it only calls
triggerManualReconnect('visibility-resume') and returns (remove the
runLifecycleRecovery call), leaving runLifecycleRecovery('visibilitychange',
1500) for the non-error path — this keeps triggerManualReconnect as the sole
immediate reconnection path and prevents ensureRegistered from being driven by
the short 500ms recovery timer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d991e2c5-f6bf-4076-8d0d-f875c2ea10a2
📒 Files selected for processing (2)
src/components/layout/header.tsxsrc/hooks/use-telnyx.ts
| const handleStatusChange = useCallback(async (newStatus: AgentStatus): Promise<boolean> => { | ||
| if (newStatus === 'available' && !isRegistered) { | ||
| showPresenceToast('Cannot go available — phone not connected. Please refresh.', 'error') | ||
| return false | ||
| showPresenceToast('Phone reconnecting…', 'warning') | ||
| reconnect() | ||
|
|
||
| const recovered = await waitForRegistration() | ||
| if (!recovered) { | ||
| showPresenceToast('Cannot go available — phone not connected. Click Agent Diagnostic → Reconnect.', 'error') | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: 'go-available' reason is never used.
The reconnect() function (defined at lines 1200-1209 in the hook) always passes 'manual-button' as the reason. While this still bypasses cooldown correctly, the 'go-available' reason added to the bypass list at line 670 is never actually triggered from this "Go Available" flow, making that part of the bypass check dead code.
If distinguishing "Go Available" reconnects from manual button clicks in logs is valuable, consider exposing the reason parameter:
// In UseTelnyxReturn interface
reconnect: (reason?: string) => void
// In the hook's reconnect callback
const reconnect = useCallback((reason = 'manual-button') => {
manualReconnectFnRef.current?.(reason)
}, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/header.tsx` around lines 93 - 103, handleStatusChange
triggers reconnect() when moving to 'available', but the reconnect
implementation always uses the literal 'manual-button' reason so the
'go-available' bypass branch is never reached; update the reconnect signature in
the UseTelnyxReturn type to accept an optional reason (e.g., reconnect:
(reason?: string) => void), change the hook's reconnect callback (where
manualReconnectFnRef is invoked) to accept a reason parameter with a
'manual-button' default, and call reconnect('go-available') from
handleStatusChange so the 'go-available' reason is actually passed through and
honored by the bypass logic.
| useEffect(() => { | ||
| if (document.visibilityState !== 'visible') return | ||
| if (connectionStatus !== 'disconnected' && connectionStatus !== 'error') return | ||
|
|
||
| reconnect() | ||
| const retryTimer = setTimeout(() => reconnect(), 2500) | ||
| return () => clearTimeout(retryTimer) | ||
| }, [connectionStatus, reconnect]) |
There was a problem hiding this comment.
Potential duplicate reconnect triggers with hook's visibility handler.
This effect runs when connectionStatus changes to 'disconnected'/'error' while the tab is visible. However, the hook (use-telnyx.ts lines 882-894) also has a handleVisibilityChange listener that triggers triggerManualReconnect('visibility-resume') when the tab becomes visible with those same statuses.
When the tab becomes visible after backgrounding:
- Hook's
handleVisibilityChangefires → callstriggerManualReconnect('visibility-resume') - This effect re-renders (if
connectionStatusis stale as 'disconnected') → callsreconnect()
Both reconnect paths will fire nearly simultaneously. While internal guards prevent duplicate init() calls, this creates redundant code paths and confusing logs.
Additionally, the 2500ms retry at line 165 may interfere with the hook's own recovery timing (500ms runLifecycleRecovery).
Consider either:
- Removing this effect and relying solely on the hook's visibility handler
- Or removing the hook's visibility-based reconnect and handling it entirely in the header
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/layout/header.tsx` around lines 160 - 167, The useEffect in
header.tsx that watches connectionStatus and calls reconnect() duplicates the
hook's visibility-based reconnect path; remove this entire effect (the useEffect
that checks document.visibilityState, connectionStatus and schedules reconnect
with setTimeout) and rely on the hook's handleVisibilityChange /
triggerManualReconnect and its runLifecycleRecovery timing instead (symbols: the
useEffect block in header.tsx, connectionStatus, reconnect; hook symbols:
handleVisibilityChange, triggerManualReconnect, runLifecycleRecovery in
use-telnyx). Ensure no other code depends on that setTimeout retry before
removing.
|
|
||
| if (connectionStatusRef.current === 'disconnected' || connectionStatusRef.current === 'error') { | ||
| void triggerManualReconnect('visibility-resume') | ||
| runLifecycleRecovery('visibilitychange', 500) | ||
| return | ||
| } | ||
|
|
||
| runLifecycleRecovery('visibilitychange', 1500) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider potential double-reconnect on visibility resume.
When the tab becomes visible with disconnected/error status, both triggerManualReconnect('visibility-resume') and runLifecycleRecovery('visibilitychange', 500) are invoked. If triggerManualReconnect completes within 500ms (resetting isReconnectingRef), ensureRegistered may trigger another reconnect attempt.
This is likely benign given the existing guards (manualReconnectInFlightRef, cooldown checks), but the dual-path could be simplified:
- Rely solely on
triggerManualReconnectfor the immediate case (it handles full reconnection) - Skip
runLifecycleRecoverywhentriggerManualReconnectis invoked
If the intent is to have ensureRegistered verify post-reconnect state, the 500ms delay may be too short for the backoff delay in triggerManualReconnect (which starts at 3s).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/use-telnyx.ts` around lines 885 - 892, When visibility resumes and
connectionStatusRef.current is 'disconnected' or 'error', avoid invoking both
triggerManualReconnect('visibility-resume') and
runLifecycleRecovery('visibilitychange', 500) to prevent a possible
double-reconnect; change the branch in the visibility handler so it only calls
triggerManualReconnect('visibility-resume') and returns (remove the
runLifecycleRecovery call), leaving runLifecycleRecovery('visibilitychange',
1500) for the non-error path — this keeps triggerManualReconnect as the sole
immediate reconnection path and prevents ensureRegistered from being driven by
the short 500ms recovery timer.
1 participant
Tab going to background drops WebRTC. Now auto-reconnects on visibility change, Go Available reconnects before failing, cooldown bypassed for explicit recovery.
Summary by CodeRabbit