chore: trace device permission state transitions#2168
Merged
Conversation
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThreads an optional Changes
Sequence DiagramsequenceDiagram
participant React as DevicesDash Hook
participant Manager as Device Manager
participant State as ManagerState
participant Devices as devices.ts Factory
participant BrowserPerm as BrowserPermission
participant Tracer as Tracer
React->>Manager: init (pass call.tracer)
Manager->>State: new ManagerState(tracer)
State->>Devices: get*BrowserPermission(tracer)
Devices->>BrowserPerm: create BrowserPermission({ tracer })
BrowserPerm->>Tracer: hold tracer reference
Note over BrowserPerm,Tracer: On permission change
BrowserPerm->>Tracer: emit("navigator.mediaDevices.<query>.permission", { previous, state })
Tracer->>Tracer: record trace event
BrowserPerm->>State: notify listeners of state change
State->>Manager: update state
Manager->>React: expose updated device/permission state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
sample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx (2)
99-111: Inconsistent tracer usage in device enumeration.The
devices$observable is created without passing the tracer (getVideoDevices()on line 101), whilepermission$correctly receivescall?.tracer. This means device enumeration won't benefit from the tracer while permission state transitions will.For consistency, consider passing the tracer to
getVideoDevicesas well:♻️ Suggested fix
function useVideoDevices() { const call = useCall(); - const devices$ = useMemo(() => getVideoDevices(), []); + const devices$ = useMemo(() => getVideoDevices(call?.tracer), [call]); const permission$ = useMemo( () => getVideoBrowserPermission(call?.tracer).asObservable(), [call], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx` around lines 99 - 111, In useVideoDevices(), getVideoDevices() is called without the tracer while getVideoBrowserPermission uses call?.tracer, causing inconsistent tracing; update the devices$ creation to pass the tracer (use getVideoDevices(call?.tracer) or equivalent) so both observables use the same tracer source (references: function useVideoDevices, variable devices$, and function getVideoDevices).
113-125: Same inconsistency for audio devices.Similarly,
getAudioDevices()on line 115 should receive the tracer for consistent tracing coverage.♻️ Suggested fix
function useAudioDevices() { const call = useCall(); - const devices$ = useMemo(() => getAudioDevices(), []); + const devices$ = useMemo(() => getAudioDevices(call?.tracer), [call]); const permission$ = useMemo( () => getAudioBrowserPermission(call?.tracer).asObservable(), [call], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx` around lines 113 - 125, The useAudioDevices hook calls getAudioDevices() without passing the tracer, causing inconsistent tracing; update useAudioDevices (the devices$ memo) to call getAudioDevices with the current tracer (e.g., getAudioDevices(call?.tracer)) so it mirrors the permission$ call that uses getAudioBrowserPermission(call?.tracer), keeping tracing consistent across audio device and permission observables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/client/src/devices/devices.ts`:
- Line 319: The recursive call in getVideoStream is missing the tracer
parameter, so retries aren't traced; update the recursive invocation
getVideoStream(relaxedConstraints) to pass the same tracer used by the current
call (mirror how getAudioStream forwards tracer) so the retry path receives and
propagates the tracer context.
---
Nitpick comments:
In `@sample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx`:
- Around line 99-111: In useVideoDevices(), getVideoDevices() is called without
the tracer while getVideoBrowserPermission uses call?.tracer, causing
inconsistent tracing; update the devices$ creation to pass the tracer (use
getVideoDevices(call?.tracer) or equivalent) so both observables use the same
tracer source (references: function useVideoDevices, variable devices$, and
function getVideoDevices).
- Around line 113-125: The useAudioDevices hook calls getAudioDevices() without
passing the tracer, causing inconsistent tracing; update useAudioDevices (the
devices$ memo) to call getAudioDevices with the current tracer (e.g.,
getAudioDevices(call?.tracer)) so it mirrors the permission$ call that uses
getAudioBrowserPermission(call?.tracer), keeping tracing consistent across audio
device and permission observables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 18fa15ee-57cc-4475-9799-797a8287bbb3
📒 Files selected for processing (7)
packages/client/src/devices/BrowserPermission.tspackages/client/src/devices/CameraManager.tspackages/client/src/devices/CameraManagerState.tspackages/client/src/devices/MicrophoneManager.tspackages/client/src/devices/MicrophoneManagerState.tspackages/client/src/devices/devices.tssample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx
jdimovska
approved these changes
Mar 20, 2026
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
2 participants
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.
💡 Overview
Enhances our tracing with information about device permission state transitions.
This should help us pinpoint device permission-related issues faster.
Summary by CodeRabbit