fix: harden dialer reconnect lifecycle + unblock Next build#342
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Telnyx lifecycle utility module and tests, integrates lifecycle-driven recovery and visibility-aware health checks into Changes
Sequence DiagramsequenceDiagram
participant App as Client App
participant Hook as useTelnyx Hook
participant Lib as Telnyx Lifecycle Lib
participant SDK as Telnyx SDK
participant State as Connection State
App->>Hook: user interaction / pageshow / visibility / socket close / sdk-error
Hook->>Lib: shouldRunInteractionRecovery(now,lastAt)?
Lib-->>Hook: allow / reject
alt allow
Hook->>Lib: getLifecycleRecoveryDelay(reason)
Lib-->>Hook: delay(ms)
Hook->>Hook: schedule runLifecycleRecovery (timeout)
Hook->>SDK: runLifecycleRecovery() (immediate + scheduled)
SDK->>State: attempt reconnect / re-register
State-->>Hook: update connection/registration status
Hook->>Hook: ensureRegistered('makeCall') (await)
alt success
Hook-->>App: makeCall resolves true
else failure
Hook-->>App: show error toast / return false
end
else throttled
Hook-->>App: skip recovery (cooldown)
end
App->>Hook: periodic health-check loop
Hook->>Lib: getRegistrationHealthCheckIntervalMs(visibilityState,...)
Lib-->>Hook: interval(ms)
Hook->>Hook: schedule next health-check (rescheduled timeout)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Comment |
![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/hooks/use-telnyx.ts`:
- Around line 878-883: The visibilitychange handler should clear and re-arm the
periodic registrationCheckTimer so hidden/visible cadence updates take effect
immediately: in useTelnyx move registrationCheckTimer declaration to an outer
scope accessible by handleVisibilityChange, then inside handleVisibilityChange
clear any existing registrationCheckTimer and set a new timeout using
getLifecycleRecoveryDelay('visibilitychange') (but only call
runLifecycleRecovery immediately when visibilityState === 'visible'); ensure you
reference and update registrationCheckTimer, handleVisibilityChange,
runLifecycleRecovery, and getLifecycleRecoveryDelay so the timer is always
rescheduled on every visibilitychange.
- Around line 1134-1157: The makeCall callback currently fires an inner async
IIFE and returns void, so callers can't know whether a call was actually
started; change makeCall to be an async function that returns a Promise<boolean>
(or Promise<Call | null> if you prefer the call object) and resolve true when
clientRef.current.newCall succeeds (and set callRef.current), or resolve false
when recovery fails or an exception occurs (and still call showPresenceToast on
failure). Update the public type UseTelnyxReturn.makeCall to match the new
Promise return type, adjust the implementation to await
ensureRegisteredRef.current/runLifecycleRecoveryRef.current before creating the
call and then return the appropriate boolean/result, and update all callers
(e.g., dialer/page.tsx, softphone.tsx, lead-table.tsx) to await the result and
roll back UI/persistence when it returns false.
In `@src/middleware.ts`:
- Around line 8-11: The middleware matcher currently applies to all routes
(export const config.matcher) causing unauthenticated API calls to be redirected
by updateSession; change the matcher to explicitly exclude API routes (i.e.,
ensure any path beginning with /api is omitted from the matcher along with the
existing _next/static, _next/image, favicon.ico and asset extensions) so
middleware won't run for /api/* and individual API route handlers can enforce
authentication themselves.
🪄 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: 722a28e1-b5dd-46e2-9b0c-a5ea6ef864a1
📒 Files selected for processing (4)
src/__tests__/telnyx-lifecycle.test.tssrc/hooks/use-telnyx.tssrc/lib/telnyx-lifecycle.tssrc/middleware.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/hooks/usePresence.ts`:
- Around line 18-20: The exported function shouldSendPresenceHeartbeat is
trivial and only used internally (called with presenceLeaderRef.current) while
external modules like AgentDiagnostic and TelnyxWrapper consume isLeader
directly; either remove the export and make shouldSendPresenceHeartbeat a
private/internal helper inside usePresence.ts (so only the hook uses it), or
keep the export but add a brief comment stating it exists solely for unit
testing; update references to use presenceLeaderRef.current where needed and
ensure no external code depends on shouldSendPresenceHeartbeat before removing
the export.
🪄 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: 3e87a04d-79f1-4914-b702-22e3725151e6
📒 Files selected for processing (2)
src/__tests__/use-presence-heartbeat.test.tssrc/hooks/usePresence.ts
![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: 02c8c8e5bc
ℹ️ 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".
|
|
||
| export const config = { | ||
| matcher: [ | ||
| '/((?!api|_next/static|_next/image|favicon.ico|.*\\.(?:svg|png|jpg|jpeg|gif|webp)$).*)', |
There was a problem hiding this comment.
The shim now excludes all /api/* paths from middleware execution, which means those requests no longer go through updateSession and the auth gate implemented in middleware.ts (that previously exempted only specific API prefixes like /api/webhooks, /api/auth, etc.). In practice this can let unauthenticated API calls bypass middleware-based protections and skip session refresh behavior for API traffic; the shim matcher should mirror the root matcher instead of globally excluding api.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/hooks/usePresence.ts`:
- Around line 412-413: The BroadcastChannel leader-win path sets
staleCheckNeededRef.current = true and calls sendHeartbeat(), but the
localStorage fallback leader-acquisition path does not, causing up-to-30s delay;
extract or add a shared "became leader" handler (e.g., create a
becameLeader/handleBecameLeader function) and invoke it from both the
BroadcastChannel leader branch and the localStorage fallback branch so that
staleCheckNeededRef.current is set and sendHeartbeat() is called immediately
when a tab becomes leader (respect existing gating logic around
isLeader/isLeaderRef).
🪄 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: ec65c47d-1c54-473a-aa77-2a9d9b47e542
📒 Files selected for processing (6)
src/app/(app)/dialer/page.tsxsrc/components/leads/lead-table.tsxsrc/components/phone/softphone.tsxsrc/hooks/use-telnyx.tssrc/hooks/usePresence.tssrc/middleware.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 210c48c798
ℹ️ 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".
| void ensureRegistered('periodic').finally(() => { | ||
| scheduleRegistrationHealthCheck() | ||
| }) |
There was a problem hiding this comment.
This reschedules the periodic registration timeout unconditionally after ensureRegistered('periodic') finishes, but handleVisibilityChange also starts a new timer whenever the tab visibility changes. If visibility changes while a periodic check is already in flight, you can end up with two independent timeout chains, which then keep running and cause redundant ensureRegistered calls/reconnect attempts. This is reproducible under slow network checks plus quick hide/show tab transitions.
Useful? React with 👍 / 👎.
Merges PR #342 after CodeRabbit approval.
1 participant
Summary
n- harden Telnyx reconnect lifecycle on the dialer pathn- add a functional src/middleware shim so Next 16 build stops failing on ./src/middleware.tsnn## Validationn- npm run build ?n- npx vitest run src/tests/telnyx-lifecycle.test.ts ?n- npm test ?n- npm run test:coverage ?`n- npm run lint ?? blocked by pre-existing repo-wide eslint issues unrelated to this changeSummary by CodeRabbit
New Features
Bug Fixes
Tests