fix(voip): gate incoming answer on microphone permission#7354
fix(voip): gate incoming answer on microphone permission#7354diegolmello wants to merge 3 commits into
Conversation
Request mic access before accept() so denied permissions hang up immediately instead of stalling signaling until the 10s timeout. Adds settings redirect for permanent denial on both platforms. Co-authored-by: Cursor <cursoragent@cursor.com>
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughRefactors VoIP microphone permission handling to return { granted, canAskAgain } instead of a boolean, adds showVoipMicrophoneDeniedAlert, implements cross-platform permission requests, and updates MediaSessionInstance flows and tests to use the new contract. ChangesVoIP Permissions API Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
Co-authored-by: Cursor <cursoragent@cursor.com>
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/methods/voipCallPermissions.ts`:
- Around line 13-37: The requestVoipCallPermissions function currently calls
PermissionsAndroid.request, Audio.getPermissionsAsync, and
Audio.requestPermissionsAsync without handling rejections; wrap the entire body
of requestVoipCallPermissions in a try/catch, catch any thrown errors from
PermissionsAndroid, Audio, or permission calls, log the error (e.g. via
console.error or the app logger) including which API failed, and return a safe
denied result { granted: false, canAskAgain: false } on failure so callers get a
controlled denial UX; ensure references to PermissionsAndroid, Audio, and
PermissionStatus remain intact.
🪄 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: CHILL
Plan: Pro
Run ID: bcdcc091-13c0-4ccd-8875-cbb71589521d
📒 Files selected for processing (5)
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsxapp/lib/methods/voipCallPermissions.test.tsapp/lib/methods/voipCallPermissions.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsxapp/lib/methods/voipCallPermissions.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/methods/voipCallPermissions.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode and baseUrl set to app/ for import resolution
Files:
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsxapp/lib/methods/voipCallPermissions.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/methods/voipCallPermissions.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Prettier with tabs, single quotes, 130 char width, no trailing commas, arrow parens avoid, bracket same line
Use@rocket.chat/eslint-configbase with React, React Native, TypeScript, Jest plugins
Files:
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsxapp/lib/methods/voipCallPermissions.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/methods/voipCallPermissions.test.ts
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Reusable UI components should be placed in app/containers/ directory
Files:
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx
app/lib/services/voip/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
VoIP service implementation should use Zustand stores (not Redux) and include native CallKit (iOS) and Telecom (Android) integration in app/lib/services/voip/
Files:
app/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsxapp/lib/methods/voipCallPermissions.tsapp/lib/services/voip/MediaSessionInstance.test.tsapp/lib/services/voip/MediaSessionInstance.tsapp/lib/methods/voipCallPermissions.test.ts
🔇 Additional comments (5)
app/lib/methods/voipCallPermissions.test.ts (1)
1-144: LGTM!app/lib/services/voip/MediaSessionInstance.ts (1)
36-37: LGTM!Also applies to: 176-181, 238-241
app/lib/services/voip/MediaSessionInstance.test.ts (1)
132-137: LGTM!Also applies to: 248-249, 700-709, 877-915
app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx (1)
142-142: LGTM!app/lib/methods/voipCallPermissions.ts (1)
3-3: ⚡ Quick winExpo-av
Audio.getPermissionsAsync()status can be compared toexpo-camera’sPermissionStatus
Audio.getPermissionsAsync()(expo-av) andPermissionStatusused byexpo-cameraboth come from Expo’s sharedexpo-modules-coreenum, so the comparison isn’t a brittle cross-package contract (app/lib/methods/voipCallPermissions.ts, lines 3 and 29). Optionally, importPermissionStatusfromexpo-modules-corefor clarity.> Likely an incorrect or invalid review comment.
diegolmello
had a problem deploying
to
upload_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.73.0.108985 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNS__32oABVGneMrdnlkqLu9q2iqsMyqMSuQ0KZ3r7zS-sBD-tSngWZdakKZ-QKi9t3Ydday7lCTV9-jH_S6 |
|
iOS Build Available Rocket.Chat 4.73.0.108986 |
diegolmello
left a comment
diegolmello
left a comment
There was a problem hiding this comment.
Code review — incoming VoIP mic-permission gate
Solid fix on the happy path with good unit coverage. The main risks are on the error / background path of the incoming-answer flow (where the new await was inserted), plus some duplication with the existing audio-recording permission code in MicOrSendButton. Inline comments below, roughly highest-severity first.
Verified non-issues: PermissionStatus from expo-camera matches expo-av's status enum (both re-export from expo-modules-core; same pattern already ships in MicOrSendButton); Android result !== NEVER_ASK_AGAIN covers all return values; all i18n keys (Ok/Cancel/Settings/…) exist; no dead imports.
- requestVoipCallPermissions never rejects: a thrown permission check is treated as denied + logged, so a locked/backgrounded answer ends promptly instead of hanging until the 10s signaling timeout - answerCall dedupes concurrent invocations per callId (in-flight Set, cleared in finally) to restore the idempotency its doc comment claims - simplify the iOS permission branch and collapse the denied-mic alert variants into a single Alert.alert call - cover the iOS live-prompt denial and permission-throw paths; tighten the denied-answer test to assert the reject() wire signal and native teardown
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
|
iOS Build Available Rocket.Chat 4.73.0.109006 |
diegolmello
requested a deployment
to
upload_android
GitHub Actions
Waiting
|
Android Build Available Rocket.Chat 4.73.0.109005 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNR8XgVARz7XAWRwuK-OKKoa5YUPW95EyR1tcy2r1yNPkpXkXKJXSDjxVVcwH2h-VZajvcqCVesLuThozELV |
1 participant
Proposed changes
Incoming VoIP calls could hang until the 10s signaling timeout when microphone permission was not granted. The accept path now requests mic permission before
accept()runs on both iOS and Android.When permission is denied, the call is hung up immediately and the user sees an appropriate alert. Permanent denial offers a shortcut to OS settings (same pattern as audio message recording).
Outgoing calls use the same improved denial UX.
requestVoipCallPermissionsnever rejects — a failing permission check (e.g. an Androidrequestthrow when a call is answered locked/backgrounded) is logged and treated as denied, so the call ends promptly instead of re-introducing the hang.answerCallis also de-duplicated percallId(an in-flight guard cleared infinally) so concurrent accept paths can't double-accept or double-end.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1139
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
requestVoipCallPermissionshelper.answerCallidempotency guard, simplified permission branch, collapsed denial-alert variants, and expanded tests (iOS live-prompt denial, permission-throw paths, concurrency, denied-answer wire signal).MicOrSendButton(shared alert + microphone-request helper) is tracked in NATIVE-1198; the locked/backgrounded iOS prompt limitation is inherent to a reactive gate and left to a future proactive init-time gate.Made with Cursor
Summary by CodeRabbit
Improvements
New Features
Bug Fixes
Tests