fix(call): device disconnect recovery — sequential pickers, speaker auto-recovery, picker key fix - #1198
Merged
Merged
Conversation
- Keep Tray (Fix A/V button) visible when a device error fires mid-call instead of replacing the entire call UI with the error overlay - Add Dismiss button to UserMediaError so users can clear the error without a full page reload - Show browser-specific permission guidance (with screenshot images) when dailyErrorType === "permissions", replacing the generic steps - Auto-reload when user re-grants camera/mic permissions in browser settings - Extract shared useGetMicCameraPermissions hook and PermissionDeniedGuidance component into PermissionRecovery.jsx (shared by setup flow + in-call) - Add 24 component tests (DEVRECOV-001–008) across chromium/firefox/webkit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…#1190) When Daily fires camera-error or mic-error with dailyErrorType "not-found" (device unplugged mid-call), show a device picker instead of generic steps, letting users switch to an available device without a full page reload. - Add DevicePicker component in UserMediaError: enumerates available devices and shows a select + "Switch to this device" button - Add handleSwitchDevice in VideoCall: calls setInputDevicesAsync and updates player device preference on success - Make UserMediaError outer container scrollable (overflow-y-auto) so tall error content (e.g. picker + buttons) doesn't overflow the flex container - Add _setInputDevicesCalls tracking to MockDailyProvider for test assertions - Wrap MockDailyProvider contextValue + mergedDevices in useMemo to avoid unnecessary re-renders - Add DEVRECOV-009 to DEVRECOV-011 tests covering device picker behavior Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Change trace from 'on-first-retry' to 'on' locally so Playwright UI mode captures trace data for all test runs (not just retries). CI keeps the efficient 'on-first-retry' setting via the CI env var. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nd permission revocation (Issue #1190) TDD implementation of 4 recovery workflows: - W5: Fatal Daily `error` event shows Connection lost / Removed / Expired UI with contextual Rejoin button - W6: Network interruption shows non-blocking "Reconnecting..." banner, clears on reconnect - W1 mid-call: Permission revocation shows proactive PermissionDeniedGuidance overlay, auto-reloads on re-grant - Sentry: Fix A/V button click sends error-level Sentry message (philosophy: click = detection failure) 11 new Playwright component tests, all passing across Chromium/Firefox/WebKit. Includes RECOVERY-PLAYBOOK.md documenting all 14 recovery workflows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ecovery (Issue #1190) TDD implementation of 3 additional recovery workflows: - W4: Device reconnected auto-recovery — listens for `devicechange` event during not-found errors, auto-switches to newly available device - W2: Auto-fix before picker — when a not-found error fires with exactly one alternative device available, auto-switches silently; shows picker only when multiple devices available - W7: Proactive track monitoring — polls track readyState every 5s, auto-recovers ended tracks via setInputDevicesAsync 9 new tests (WF4-001 to WF4-003, WF2-001 to WF2-003, WF7-001 to WF7-003). Updated existing DEVRECOV and ERR tests to mock enumerateDevices after mount for cross-browser compatibility (webkit resets mocks during CT mount). 398/399 tests pass across Chromium/Firefox/WebKit (1 pre-existing flaky). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ll workflows Mark W2, W4, W5, W6, W1 mid-call permission revocation, and Sentry on Fix A/V as fully implemented. Update routing matrix and coverage summary. Note the distinction between test-file "W7" (proactive track monitoring) and playbook W7 (network degraded UI). Add WF4/WF5/WF6/WF7/WF1-MID/WF2/WF-SENTRY test IDs to the test plan section. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- CLAUDE.md: add AI model selection guidance (Sonnet default, Opus on request) - playwright.config.mjs: serve client/public as publicDir in component tests - FAILURE-MODES.md: new doc enumerating Daily call failure modes with coverage gaps - GetPermissions.jsx: remove stray blank line - test-results/.last-run.json: delete unintentionally tracked test artifact Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…own-cause copy - Remove click-time Sentry.captureMessage from openFixAV (low signal if user cancels without submitting) - Keep submit-time reportedAVError capture which includes the user's selected issues and full diagnostic snapshot in the same event - Update WF-SENTRY-001 test to verify: click alone sends nothing, submission sends reportedAVError with userReportedIssues in extra - Change "No specific cause identified" to friendlier "We weren't able to pinpoint the problem" with reassuring guidance copy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create three reusable wrapper components to unify error/recovery UI patterns: - Modal (components/): white card on dark scrim, replaces 5 ad-hoc modal implementations (FixAV, ReportMissing, Check-in, IdleProvider, gesture overlay) - Toast (components/): fixed bottom-right notification with variant colors, replaces hard-coded toasts in ReportMissing - CallBanner (call/): absolute top banner for non-blocking status messages Reclassify error UIs by severity: - Fatal errors (call dead): replace call content with white card overlay - Device errors (remote tracks still working): Modal over call, preserving other participants' video instead of unmounting <Call> - Permission revoked mid-call: Modal over call - Network interrupted: non-blocking CallBanner Also switch all error UIs to white/light card style for consistency (previously mixed dark glassmorphism, white, and slate-800 cards). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gnostic clutter - Add showCloseButton prop to Modal (defaults true, renders X in top-right) - Remove diagnostic info (camera/mic status, device counts) from UserMediaError UI (data still collected and sent to Sentry, just not shown to participants) - Remove Dismiss button from UserMediaError (X close button replaces it) - Update DEVRECOV-003/004 tests to use modal close button Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…1190) - Refactor deviceErrorCopy to key on [deviceType][dailyErrorType] instead of just deviceType, giving each error cause its own title and steps: - permissions → "Camera access denied" / "Microphone access denied" - in-use → "Camera in use" / "Microphone in use" - not-found → "Camera disconnected" / "Microphone disconnected" - constraints → "Camera unavailable" / "Microphone unavailable" - unknown → "Camera problem" / "Microphone problem" - Add priority ordering to setDeviceError so higher-priority errors (permissions > in-use > not-found > constraints > unknown) are not overwritten by lower-priority ones arriving later - Add DEVRECOV-012/013 tests verifying priority behavior - Update all existing test assertions for new cause-specific titles Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…itle When both camera-error and mic-error fire with the same dailyErrorType (e.g. both "permissions"), the modal now shows "Camera and microphone access denied" instead of just the last device's error. - Add combined titles for each dailyErrorType in the default copy section - Merge logic in setDeviceError detects same-cause, different-device errors - Add DEVRECOV-014 test verifying the merge behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update file-level comment with current error title/priority design - Rewrite DEVRECOV-001/002 descriptions (modal overlay, not content replacement) - Update DEVRECOV-004 description (X close button, not Dismiss) - Remove stale "Currently FAILS" markers from DEVRECOV-005/006/007 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… flow Both the Permissions API onchange listener and Daily's camera-error/mic-error events now route through setDeviceError with dailyErrorType: "permissions". This eliminates the separate permissionRevoked state and its dedicated Modal, ensuring all permission denial errors show the same UserMediaError UI with cause-specific titles and PermissionDeniedGuidance. - Remove permissionRevoked state variable and its Modal from render - Change onchange "denied" handler to call setDeviceError instead - Remove onchange "granted" handler (UserMediaError's permission monitoring hook handles auto-reload when permissions flip back to granted) - Remove unused PermissionDeniedGuidance import from VideoCall.jsx - Update WF1-MID-001/002 tests to check for unified device error modal Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…t found When device alignment detects a fallback match (preferred device not found by ID or label), show the device picker instead of silently using the first available device — but only when 2+ alternatives exist. Single-device fallback remains silent to avoid unnecessary prompts. Also renames "disconnected" to "not available" in error titles and picker text, since this now covers both mid-call disconnection and start-of-call scenarios. Issue #1190 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the >1 device guard — show the picker even when only one alternative exists. This gives users situational awareness that we're switching to a different device than expected (e.g., built-in camera instead of their preferred external webcam), rather than silently using whatever is available. Issue #1190 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Brief network blips no longer flash a "Reconnecting..." banner. The banner only appears if the interruption persists for 5+ seconds — long enough that participants would notice something is wrong. Reconnection within the grace period cancels the timer silently. Added WF6-002 test for the grace period (brief interruption never shows banner) and WF6-002b for clearing the banner after it appears. Issue #1190 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the W2 silent auto-switch logic that switched to a single available device without user knowledge. The picker now always shows for not-found errors, consistent with the W11 alignment fallback behavior — users should always be aware when we're using a different device than expected. Updated WF2-001 and WF2-002 to assert the picker appears rather than a silent switch, and updated the playbook to document the design rationale. Issue #1190 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the failed, unfixable, partial, unknown, and cant-see states that were previously untested. Each test monkey-patches the mock call object to exercise a specific branch of generateRecoverySummary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… root The playwright/.gitignore already covers these under playwright/, but test artifacts were landing at the repo root and showing as untracked. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
useAudioContextMonitor and useAutoDiagnostics belong alongside eventLogger.js in call/hooks/. Update VideoCall.jsx imports and fix the relative path for collectAVDiagnostics inside useAutoDiagnostics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- UserMediaError: remove unused onDismiss prop and deviceSurvey state (fixes no-unused-vars lint errors; dismissal is handled by parent Modal) - Toast: add role/aria-live for screen reader announcements (assertive for error/warning, polite for info/success) - useGetMicCameraPermissions: null onchange handlers on unmount to prevent memory leaks when UserMediaError modal is dismissed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Running all browsers serially in one job was exceeding the 15-minute limit. Three parallel jobs (chromium / firefox / webkit) each install only their own browser and run with --project=<browser>, so total wall-clock time becomes max(chromium, firefox, webkit) rather than their sum. fail-fast: false lets all three complete even if one fails. Artifact names are now browser-scoped to avoid upload conflicts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Limit push trigger to main only (PR branches were firing both push and pull_request events, creating 6 redundant parallel jobs) - Add --workers=2 to override the CI=1 default; GitHub Actions runners have 2 cores and the mocked tests are lightweight enough to parallelize Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Firefox suspends AudioContext on page load without a user gesture, causing the "Enable audio" prompt to appear after 800ms regardless of setSpeaker outcome. SPEAKER-005 tests speaker alignment behaviour, not AudioContext suspension, so it should mock AudioContext the same way SPEAKER-006 already does (state: 'running'). Also add `media.autoplay.default: 0` to Firefox Playwright prefs so headless Firefox allows AudioContext auto-resume without a user gesture, making the test environment consistent with a focused Firefox tab. The production code is correct: showing the AudioContext prompt in Firefox is intentional. The test was simply under-isolated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same root cause as SPEAKER-005: Firefox suspends AudioContext without a prior user gesture, causing the gesture overlay to appear after 800ms and block clicks on the Fix A/V button (90s timeout on first attempt in CI). Adding the AudioContext mock (state: 'running') and hasFocus mock isolates the test to its intended subject — Sentry reporting on Fix A/V submission — and reduces test time from 90s+ to ~4s. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The pref controls media element autoplay policy, not AudioContext suspension tied to page focus. In CI headless Firefox, document.hasFocus() returns false, so useAudioContextMonitor fires the overlay immediately — before the 800ms timer is even reached. The pref has no effect on this code path. The correct fix for each affected test is to mock window.AudioContext (state: 'running') and document.hasFocus, which we've done in SPEAKER-005 and WF-SENTRY-001. Remove the pref to avoid misleading future readers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tests from Firefox autoplay policy In headless Firefox, document.hasFocus() returns false, causing useAudioContextMonitor to show the gesture overlay immediately on mount. This was blocking clicks in tests unrelated to AudioContext. Add a default "running" AudioContext mock + document.hasFocus=()=>true in the beforeMount hook (playwright/index.jsx) so every test is isolated from browser autoplay policies by default. Tests that deliberately test AudioContext behaviour (AUDIO-*, FIXAV-011, FIXAV-014, PERM-004) opt out by setting window.__customAudioContext=true in their page.evaluate() block before mount(), which prevents the beforeMount default from overriding their custom suspended mock. Remove the now-redundant per-test mocks from SPEAKER-005, SPEAKER-006, and WF-SENTRY-001 — they are covered by the global default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ole serialization Playwright's ConsoleMessage.text() serializes object arguments as JSHandle@object in Firefox, causing SUB-006 to fail when checking the log for the participant ID. Switching to Object.keys().join() produces a consistent string across all browsers and is also more readable in production logs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
## AGC constraint fix
setInputDevicesAsync() only accepts device IDs — it does not accept
MediaTrackConstraints. The previous audioSource: {...} call was silently
ignored (Daily warned "Received unexpected audioDeviceId"). Switch to
updateInputSettings({ audio: {...} }) which is the correct API for
applying MediaTrackConstraints mid-call. Removes the TEMP marker since
this is the intended approach (still evaluating the strategy).
## Render storm fix
When a mic is unplugged in Firefox, Daily queues mic-error events while
the page is unfocused, then flushes them all (~400+) when focus returns.
Each event called setDeviceError() with a new object reference, which
triggered the recordError useEffect in UserMediaError on every render,
causing console.error and Sentry.captureMessage to fire hundreds of times
and ultimately hitting React's "Maximum update depth exceeded" limit.
Add a same-type dedup guard: if the same error (type + dailyErrorType)
is already displayed, return the existing prev state object rather than a
new one, suppressing the cascade of redundant re-renders.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… storms Verifies that firing 50 rapid mic-error or camera-error events (simulating Firefox's event-flush on tab focus-regain) produces exactly one error modal and at most 2 console.error calls, not 50+. Also asserts no "Maximum update depth exceeded" React error appears. These tests would have caught the setDeviceError dedup regression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssue #1195 Both setInputDevicesAsync({ audioSource: constraints }) and updateInputSettings({ audio: constraints }) were tried and confirmed non-functional. Reverts to the original setInputDevicesAsync call (which silently no-ops) while the correct approach is investigated. Detailed notes left in code pointing to #1195. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d-call When a webcam+mic combo or audio output device is unplugged during a call, the OS silently auto-switches to a fallback device. Previously only Daily events (camera-error/mic-error) showed a picker, and speaker disconnects had no UI at all. Changes: - alignSpeaker(), alignCamera(), alignMic(): detect fallback match (preferred device gone) and show the device picker BEFORE the "skip if already using" guard. The old ordering meant currentDevice === fallback.deviceId → early return, so the picker never appeared when the OS had already auto-switched. - All three alignment functions pass pickerDevices directly in the error object (from devices.speakers/cameras/microphones) so the picker renders immediately without relying on enumerateDevices() returning audiooutput (unreliable in webkit headless). - Storm guard: loggedUnavailable*Ref debounces repeated setDeviceError calls when devices updates rapidly (e.g. webcam reconnect initialising), preventing render storms and "Maximum update depth exceeded" crashes. - Storm guard refs reset via useEffect whenever deviceError clears, so future disconnects of the newly selected device can show the picker again. - handleSwitchDevice: added speaker case (setSpeaker + save speakerId/label); resets loggedUnavailable*Ref on switch. - Modal onClose is null for not-found picker errors so users cannot silently dismiss to an unknown fallback device by clicking outside. - UserMediaError: uses error.pickerDevices when present (alignment path), falls back to enumerateDevices() result for Daily-event-path errors. Tests added: - DEVRECOV-015/016: speaker not found shows picker; selecting clears error - DEVRECOV-017: mic alignment fallback shows picker (OS auto-switched case) - DEVRECOV-018: camera alignment fallback shows picker (OS auto-switched case) - WF-STORM-003: rapid alignment fallback calls do not cause render storm - ERR-Breadcrumb test updated: fallback no longer logs addBreadcrumb (breadcrumbs are for successful id/label switches only) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…und error When dailyErrorType is "not-found" but pickerDevices is empty (no alternatives enumerated), the previous code fell through to the generic steps branch, which is empty for not-found copy, leaving only a title and "Reload and retry" with no explanation. Now shows: "No alternative devices were detected. Plug in a device and reload to retry." Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a monitor with camera and speakers was unplugged, camera-error and
speaker-error both fired with dailyErrorType "not-found". The merge logic
collapsed them into { type: null, pickerDevices: [speakers] }, which:
- showed the title "Camera and microphone not available" (wrong)
- showed a DevicePicker labelled "microphone" listing speaker devices
- called setInputDevicesAsync({ audioDeviceId: speakerDeviceId }) on
switch, which threw, leaving the modal permanently open
Fix:
- Move DEVICE_ERROR_PRIORITY / DEVICE_TYPE_PRIORITY outside the component
(avoids useCallback dep-array warning)
- Merge only applies to camera-error + mic-error pairs (isCameraMicPair
guard); speaker-error is never merged
- Add DEVICE_TYPE_PRIORITY: speaker-error = 1, camera/mic = 2 so a
camera/mic error always wins over a concurrent speaker error
- Change final priority comparison from >= to > so equal-priority new
errors don't displace existing ones (merge handles same-priority
camera+mic pairs before this check)
Test: DEVRECOV-019 in VideoCall.speakerRecovery.ct.jsx
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…very
Replace single deviceError state with three independent states
(cameraError, micError, speakerError) so multiple simultaneous
disconnects show pickers in sequence (camera → mic → speaker)
rather than merging into a single ambiguous error. Includes:
- keepPrev helper + DEVICE_ERROR_PRIORITY guards dedup/priority
- Fix 1: auto-dismiss not-found modal when preferred device returns
via successful non-fallback alignment on any of the three channels
- Fix 2: extend W4 devicechange handler to cover speaker-error via
audiooutput enumeration and devices.setSpeaker()
- key={pickerDeviceType} on DevicePicker to force remount when the
error type changes (prevents stale selectedId bug)
- Tests: DEVRECOV-014 updated for sequential behavior; DEVRECOV-020
(mic+speaker), DEVRECOV-021 (camera+mic+speaker), WF4-004 (speaker
devicechange auto-recovery) added in VideoCall.speakerRecovery.ct
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Implements improved in-call recovery when audio/video devices disconnect (Issue #1190), including sequential per-device pickers, speaker output auto-recovery, and a picker remount fix to prevent stale selection loops. It also expands the Playwright component-test harness (multi-browser, default AudioContext mock) and adds extensive new recovery workflow tests and documentation.
Changes:
- Refactors device error handling to separate camera/mic/speaker error states and show recovery UI sequentially, with speaker reconnect support.
- Introduces shared UI/permission recovery utilities (
Modal,Toast,PermissionRecovery,CallBanner) and integrates them into call/setup flows. - Enables multi-browser Playwright CT execution and adds new component tests + recovery playbook docs.
Reviewed changes
Copilot reviewed 33 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test-results/.last-run.json | Removes committed Playwright run artifact. |
| playwright/playwright.config.mjs | Enables Firefox/WebKit projects; updates trace behavior; serves client public assets for CT. |
| playwright/mocks/MockDailyProvider.jsx | Adds tracking/spies + memoization to stabilize CT context values and device functions. |
| playwright/index.jsx | Adds default AudioContext mock for Firefox/headless isolation (opt-out via __customAudioContext). |
| playwright/component-tests/video-call/mocked/VideoCall.speakerRecovery.ct.jsx | New CT suite for speaker/output disconnect + recovery + sequencing scenarios. |
| playwright/component-tests/video-call/mocked/VideoCall.recoveryWorkflows.ct.jsx | New CT suite covering recovery playbook workflows (fatal errors, network banner, W4/W7 storm prevention, etc.). |
| playwright/component-tests/video-call/mocked/VideoCall.deviceRecovery.ct.jsx | New CT suite focused on device error UI, picker behavior, priority, and permission guidance. |
| playwright/component-tests/video-call/mocked/Speaker.ct.jsx | Removes per-test AudioContext mocking (now centralized in beforeMount). |
| playwright/component-tests/video-call/mocked/PermissionMonitoring.ct.jsx | Opts out of default AudioContext mock for tests that control AudioContext state. |
| playwright/component-tests/video-call/mocked/FixAV.ct.jsx | Adds non-happy-path Fix A/V modal state coverage; opts out of default AudioContext mock in relevant tests. |
| playwright/component-tests/video-call/mocked/ErrorReporting.ct.jsx | Updates expectations for new not-found picker behavior and Sentry messages. |
| playwright/component-tests/video-call/mocked/DeviceAlignmentLogs.ct.jsx | Updates expected Sentry message text for fallback → picker behavior. |
| playwright/component-tests/video-call/mocked/AudioContext.ct.jsx | Opts out of default AudioContext mock to keep AudioContext tests deterministic. |
| playwright/component-tests/video-call/TEST-PROGRESS.md | Updates test progress counts and notes multi-browser enablement. |
| playwright/component-tests/video-call/RECOVERY-PLAYBOOK.md | Adds a detailed recovery spec / playbook for call failure handling. |
| playwright/component-tests/video-call/FAILURE-MODES.md | Adds failure mode taxonomy + coverage mapping for call recovery. |
| playwright/add_tests.md | Adds additional future testing questions/ideas. |
| client/src/intro-exit/setup/GetPermissions.jsx | Reuses shared permission monitoring + denied guidance component. |
| client/src/components/Toast.jsx | Adds reusable toast component with ARIA live-region behavior. |
| client/src/components/PermissionRecovery.jsx | Adds shared permission hook + denied guidance UI used in setup and in-call errors. |
| client/src/components/Modal.jsx | Adds reusable modal wrapper used across call/setup flows. |
| client/src/components/IdleProvider.jsx | Migrates idle prompt UI to shared Modal. |
| client/src/call/utils/avRecovery.test.js | Updates expected copy for “unknown” recovery summary. |
| client/src/call/utils/avRecovery.js | Updates “unknown” recovery summary messaging. |
| client/src/call/hooks/useAutoDiagnostics.js | Fixes import path for collectAVDiagnostics. |
| client/src/call/hooks/useAudioContextMonitor.js | Adds new hook for AudioContext autoplay/focus handling + auto-resume logic. |
| client/src/call/VideoCall.jsx | Implements sequential device errors, W4/W5/W6/W7 workflows, new modal integration, and speaker recovery logic. |
| client/src/call/UserMediaError.jsx | Adds permission auto-reload, device picker flow (incl. remount key fix), and expanded diagnostics logging. |
| client/src/call/ReportMissing.jsx | Migrates report-missing UI to shared Modal + Toast. |
| client/src/call/FixAV.jsx | Migrates Fix A/V modal to shared Modal; clarifies Sentry submission semantics. |
| client/src/call/CallBanner.jsx | Adds banner component (used for reconnecting indicator). |
| client/src/call/Call.jsx | Reduces console log verbosity for subscription repair. |
| CLAUDE.md | Adds guidance about model selection. |
| .gitignore | Ignores Playwright artifacts (test-results/, playwright-report/). |
| .github/workflows/playwright_component.yml | Runs CT in matrix across chromium/firefox/webkit and uploads per-browser reports on failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| * | ||
| * Device errors (camera-error, mic-error) are shown as a floating Modal over | ||
| * the still-running call — the Call component stays mounted so remote tracks | ||
| * are preserved. The modal can be closed via its X button to return to the call. |
There was a problem hiding this comment.
The header comment says the device error modal “can be closed via its X button to return to the call,” but in the current implementation VideoCall disables onClose for dailyErrorType === "not-found", which hides the close button for the picker modal. Either update this comment to reflect the actual behavior, or re-enable closing for not-found errors if dismissal is intended.
Suggested change
| * are preserved. The modal can be closed via its X button to return to the call. | |
| * are preserved. For most error types, the modal can be closed via its X button | |
| * to return to the call; for "not-found" errors, the close button is disabled | |
| * and the modal must be resolved via other recovery flows. |
Comment on lines
+27
to
+35
| return ( | ||
| // eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions | ||
| <div | ||
| className="fixed inset-0 z-50 flex items-center justify-center bg-black/50" | ||
| onClick={handleBackdropClick} | ||
| > | ||
| <div | ||
| className={`relative mx-4 w-full ${widthClass} rounded-lg bg-white p-6 shadow-xl`} | ||
| > |
There was a problem hiding this comment.
Modal renders a full-screen overlay but doesn’t expose dialog semantics (e.g., role="dialog", aria-modal="true", and a way to associate the title via aria-labelledby). Without these, screen readers may not announce the modal correctly and users can remain in the underlying page context. Consider adding the appropriate ARIA attributes (and ideally initial focus management / Escape-to-close when onClose is provided).
Comment on lines
+699
to
+729
| if (relevantDevices.length > 0) { | ||
| const device = relevantDevices[0]; | ||
| if (isCamera) { | ||
| await callObject.setInputDevicesAsync({ | ||
| videoDeviceId: device.deviceId, | ||
| }); | ||
| setCameraError(null); | ||
| } else if (isSpeaker) { | ||
| await devices.setSpeaker(device.deviceId); | ||
| setSpeakerError(null); | ||
| } else { | ||
| await callObject.setInputDevicesAsync({ | ||
| audioDeviceId: device.deviceId, | ||
| }); | ||
| setMicError(null); | ||
| } | ||
| Sentry.addBreadcrumb({ | ||
| category: "device-recovery", | ||
| message: `Device reconnected: auto-switched ${ | ||
| // eslint-disable-next-line no-nested-ternary | ||
| isCamera ? "camera" : isSpeaker ? "speaker" : "microphone" | ||
| }`, | ||
| level: "info", | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.warn( | ||
| "[VideoCall] Device reconnection auto-recovery failed:", | ||
| err | ||
| ); | ||
| } |
There was a problem hiding this comment.
In the devicechange auto-recovery path, speaker recovery calls devices.setSpeaker() but any NotAllowedError (Safari gesture requirement) is swallowed by the catch, leaving the user stuck with the same modal and no gesture prompt. Consider detecting NotAllowedError / "user gesture" here and routing through handleSetupFailure("speaker", ...) (similar to the alignment + picker switch paths) so the unified gesture modal can recover.
Comment on lines
38
to
41
| use: { | ||
| trace: 'on-first-retry', | ||
| trace: process.env.CI ? 'on-first-retry' : 'on', | ||
| screenshot: 'only-on-failure', | ||
| video: 'retain-on-failure', |
There was a problem hiding this comment.
Local runs now always record Playwright traces (trace: 'on' when not in CI). This can significantly slow down component tests and produce large artifacts on developer machines. Consider gating this behind an env flag (e.g. PW_TRACE=1) or using a lighter default like 'retain-on-failure' outside CI.
Deliberation
|
||||||||||||||||||||||||||||
| Project |
Deliberation
|
| Branch Review |
fix/device-recovery-1190
|
| Run status |
|
| Run duration | 08m 58s |
| Commit |
|
| Committer | James Houghton |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
1
|
Skipped
|
0
|
Passing
|
21
|
| View all changes introduced in this branch ↗︎ | |
UserMediaError.jsx: - Escape apostrophe in JSX string (react/no-unescaped-entities) - Extract pickerDevices nested ternary into getPickerDevices() helper - Replace pickerDeviceType nested ternary with if/else - Fix consistent-return in useEffect (return undefined on early exit) - Extract chained JSX ternaries into renderBody() function VideoCall.jsx: - Add eslint-disable comment for intentionally-excluded player dep in join effect (adding player would cause unwanted re-joins) client/package.json: - Move @daily-co/daily-js and @daily-co/daily-react from devDependencies to dependencies (they are runtime imports) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
origin/main contained the squashed PR #1191 (all our early commits) and an independent hooks.js retry log improvement (#1196). Our branch is a superset of main's content; resolved all add/add file conflicts by taking the HEAD (branch) version which includes the new sequential modal + lint fixes added after the PR was opened. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
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
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
This PR implements device disconnect recovery improvements for Issue #1190, covering three areas:
deviceErrorstate with three independent states (cameraError,micError,speakerError). When multiple devices disconnect simultaneously (e.g. monitor with camera + speakers unplugged), pickers now appear in sequence — camera first (most critical), then mic, then speaker — rather than merging into a single ambiguous error.devicechangeevent handler to cover speaker disconnects. Previously it only handled camera and mic (viasetInputDevicesAsync). Now when a speaker error is showing and a new audio output device appears, the handler callsdevices.setSpeaker()and clears the error.key={pickerDeviceType}toDevicePickerto force remount when transitioning between device types (e.g. camera → mic). Without this,selectedIdstate from the camera picker was sent as the mic device ID, triggering a re-error loop.New tests
devicechangeauto-recovers459 tests passing across chromium, firefox, and webkit.
Test plan
npx playwright test --config playwright/playwright.config.mjs "video-call/mocked"— all passCloses #1190
🤖 Generated with Claude Code