fix(android): handle null VoiceConnection in answerIncomingCall, notify JS

[diegolmello]

Proposed changes

Android null guard: answerIncomingCall now handles the null VoiceConnection case (Telecom connection killed by the system). It cleans up Telecom (disconnectIncomingCall, cancelById), stops DDP (ddpRegistry.stopClient), and stashes a voipAcceptFailed payload via VoipModule.storeAcceptFailureForJs so JS shows a VoIP_Call_Issue toast instead of leaving the UI hanging.

TS log cleanup: Removed the // TODO: Show error message? comment from the "Call not found after accept" branch in MediaSessionInstance.answerCall. The toast is already wired through the voipAcceptFailed deep-link saga, so the TODO was stale.

Issue(s)

N/A

How to test or reproduce

1. Start an incoming VoIP call on Android.
2. Force a scenario where Telecom's VoiceConnection is gone by the time answerIncomingCall runs (e.g. system kills the connection under memory pressure, or the connection is canceled before accept completes).
3. Verify the user sees the VoIP_Call_Issue toast instead of a hanging call UI, and that DDP/Telecom state is cleaned up.

Screenshots

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Documentation update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Further comments

Files changed

• android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
  • answerIncomingCall now takes a Context parameter.
  • Null-VoiceConnection branch: calls disconnectIncomingCall, cancelById, ddpRegistry.stopClient, and VoipModule.storeAcceptFailureForJs with voipAcceptFailed = true to notify JS.
• app/lib/services/voip/MediaSessionInstance.ts
  • Cleaned up the "Call not found after accept" log; removed stale TODO comment since the toast path is already wired through the deep-link saga.

Acceptance criteria

Criterion	Status
Android null return triggers Telecom + DDP cleanup	disconnectIncomingCall, cancelById, ddpRegistry.stopClient
Android null return notifies JS	VoipModule.storeAcceptFailureForJs with voipAcceptFailed = true
User sees error message (toast)	storeAcceptFailureForJs -> VoipAcceptFailed event -> deep-link saga -> VoIP_Call_Issue toast

Summary by CodeRabbit

• Bug Fixes

  • Improved VoIP call acceptance error handling with recovery steps when calls fail to connect, including notification cancellation and call cleanup.
  • Enhanced error logging to surface warnings for failed accept flows.

• Tests

  • Updated integration test expectations to match revised VoIP warning messaging after accept failures.

[diegolmello]

Code Review — PR #7201

Files Reviewed: VoipNotification.kt, VoipCallService.kt, AndroidManifest.xml, MediaSessionInstance.ts
Issues: 0 CRITICAL/HIGH, 2 MEDIUM observations

---

[MEDIUM] answerIncomingCall signature changed — all callers need Context

File: VoipNotification.kt:305

answerIncomingCall now takes a Context parameter. The call site at line 253 passes appCtx (from the outer handleAcceptAction scope), which is correct. However, verify there are no other call sites in the codebase that call answerIncomingCall without the Context parameter.

Checked: grep search confirms only one call site in handleAcceptAction. ✅

---

[MEDIUM] Null VoiceConnection error payload is minimal

File: VoipNotification.kt:318-328

When answerIncomingCall encounters a null VoiceConnection, it creates a minimal VoipPayload with empty strings for caller, username, host, type, hostName and null for avatarUrl/createdAt. This is intentional (we don't have the original payload in the null case), but the deep-linking saga's handleVoipAcceptFailed checks params.username for navigation and params.voipAcceptFailed for the toast.

No action required — voipAcceptFailed: true ensures the toast fires even with empty navigation params.

---

Acceptance Criteria Verification

Criterion	Status	Notes
Android null return triggers state cleanup	✅	disconnectIncomingCall, ddpRegistry.stopClient, storeAcceptFailureForJs
iOS null return triggers RNCallKeep.endCall + cleanup	✅	Already in handleNativeAccept finishAccept(false) path
User sees error message (toast)	✅	storeAcceptFailureForJs → VoipAcceptFailed event → VoIP_Call_Issue toast
Both platforms behave consistently	✅	Both paths stash failure for JS recovery

---

Verdict

LGTM — All 4 acceptance criteria satisfied. Both MEDIUM observations are informational only.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Android: pass application context into the VoIP accept flow and add recovery steps when a voice connection is missing. TypeScript and tests: update warning text to indicate the call was not found after accept.

Changes

Cohort / File(s)	Summary
Android VoIP accept flow android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt	Change answerIncomingCall signature to accept Context; handleAcceptAction now calls answerIncomingCall(appCtx, payload.callId); when VoiceConnectionService.getConnection(callId) returns null, log a warning and perform recovery: disconnectIncomingCall(callId, false), cancelById(appCtx, 0), ddpRegistry.stopClient(callId), and persist an accept-failure VoipPayload for JS via VoipModule.storeAcceptFailureForJs(...).
TypeScript warning message app/lib/services/voip/MediaSessionInstance.ts	Update console warning text from "[VoIP] Call not found:" to "[VoIP] Call not found after accept:".
Integration test update app/containers/NewMediaCall/VoipCallLifecycle.integration.test.tsx	Adjust console warning allowlist and assertion to expect "[VoIP] Call not found after accept:".

Sequence Diagram(s)

sequenceDiagram
  participant Notification as VoipNotification
  participant AndroidSvc as VoiceConnectionService
  participant DDP as DdpRegistry
  participant VoipModule as VoipModule(JS)

  Notification->>Notification: handleAcceptAction(appCtx, payload)
  Notification->>Notification: answerIncomingCall(appCtx, callId)
  Notification->>AndroidSvc: getConnection(callId)
  alt connection found
    AndroidSvc-->>Notification: connection
    Notification->>AndroidSvc: onAnswer / proceed with connection
  else connection null
    AndroidSvc-->>Notification: null
    Notification->>Notification: log warning
    Notification->>Notification: disconnectIncomingCall(callId, false)
    Notification->>Notification: cancelById(appCtx, 0)
    Notification->>DDP: stopClient(callId)
    Notification->>VoipModule: storeAcceptFailureForJs(callId, payload)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding null-guard handling for VoiceConnection in answerIncomingCall with JS notification on Android.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)

248-329: ⚠️ Potential issue | 🟠 Major

Don’t let the null-connection path fall through as a successful accept.

Line 318 stores an accept failure, but answerIncomingCall() returns Unit, so Line 254 still stores the normal initial call event after native cleanup. That can make JS process both “accept failed” and “incoming call” state. Return a success/failure result from native answer and only call storeInitialEvents when Telecom actually had a connection.

Proposed control-flow fix

                 ddpRegistry.stopClient(payload.callId)
                 if (answerRequestSucceeded) {
-                    answerIncomingCall(appCtx, payload.callId)
-                    VoipModule.storeInitialEvents(payload)
+                    if (answerIncomingCall(appCtx, payload)) {
+                        VoipModule.storeInitialEvents(payload)
+                    }
                 } else {
                     Log.d(TAG, "media-calls.answer failed for ${payload.callId}; opening app for JS recovery")
                     disconnectIncomingCall(payload.callId, false)
                     VoipModule.storeAcceptFailureForJs(payload)
                 }
@@
-        private fun answerIncomingCall(context: Context, callId: String) {
+        private fun answerIncomingCall(context: Context, payload: VoipPayload): Boolean {
+            val callId = payload.callId
             val connection = VoiceConnectionService.getConnection(callId)
-            when (connection) {
-                is VoiceConnection -> connection.onAnswer()
+            return when (connection) {
+                is VoiceConnection -> {
+                    connection.onAnswer()
+                    true
+                }
                 null -> {
                     // C3 fix: null return means Telecom connection is gone (system killed it).
                     // Notify JS so the user sees an error instead of a hanging UI.
                     Log.w(TAG, "No active VoiceConnection for accepted call: $callId — notifying JS of failure")
                     val appCtx = context.applicationContext
                     disconnectIncomingCall(callId, false)
-                    cancelById(appCtx, 0)
+                    cancelById(appCtx, payload.notificationId)
                     ddpRegistry.stopClient(callId)
                     // Stash failure payload for JS: it will show error toast on getInitialEvents.
                     VoipModule.storeAcceptFailureForJs(VoipPayload(
-                        callId = callId,
-                        caller = "",
-                        username = "",
-                        host = "",
-                        type = "",
-                        hostName = "",
-                        avatarUrl = null,
-                        createdAt = null,
+                        callId = payload.callId,
+                        caller = payload.caller,
+                        username = payload.username,
+                        host = payload.host,
+                        type = payload.type,
+                        hostName = payload.hostName,
+                        avatarUrl = payload.avatarUrl,
+                        createdAt = payload.createdAt,
                         voipAcceptFailed = true
                     ))
+                    false
                 }
-                else -> Log.d(TAG, "Non-VoiceConnection for accept, callId: $callId")
+                else -> {
+                    Log.d(TAG, "Non-VoiceConnection for accept, callId: $callId")
+                    false
+                }
             }
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`
around lines 248 - 329, The null-connection path in answerIncomingCall currently
only logs and stores a failure payload but returns Unit, so finish(...) still
treats the accept as succeeded and calls VoipModule.storeInitialEvents; change
answerIncomingCall(context: Context, callId: String) to return Boolean (true
when a VoiceConnection was found and .onAnswer() called, false when null path
taken), update finish(answerRequestSucceeded: Boolean) to call
answerIncomingCall(...) and combine its Boolean result with
answerRequestSucceeded before calling VoipModule.storeInitialEvents (i.e., only
call storeInitialEvents when both the network answer succeeded and
answerIncomingCall returned true), and keep the existing null-path cleanup
(disconnectIncomingCall, cancelById, ddpRegistry.stopClient,
VoipModule.storeAcceptFailureForJs) inside the now-returning-false branch;
locate these changes around the finish function and the answerIncomingCall and
VoiceConnectionService.getConnection usages.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt`:
- Around line 70-79: The service currently returns START_STICKY for ACTION_START
which can cause Android to recreate the service with a null intent; update
VoipCallService (onStartCommand) so the ACTION_START branch returns
START_NOT_STICKY instead of START_STICKY, and modify the unknown/else branch to
call stopSelf(startId) to ensure the service is terminated when intent is
invalid (retain logging and ensure startForegroundWithNotification(callId) is
still called for ACTION_START). Use the ACTION_START symbol, the
startForegroundWithNotification(callId) call, and the onStartCommand startId
parameter to locate where to change the return value and insert
stopSelf(startId).
- Around line 48-54: The stopService static helper currently calls
context.startService(intent) which can throw on Android O+ when called from
background; change VoipCallService.stopService(context) to call
context.stopService(intent) instead so the Intent (with ACTION_STOP) is sent via
Context.stopService to stop the foreground service safely from backgrounded
contexts. Locate the stopService method in VoipCallService and replace the
context.startService call with context.stopService using the same Intent.

---

Outside diff comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 248-329: The null-connection path in answerIncomingCall currently
only logs and stores a failure payload but returns Unit, so finish(...) still
treats the accept as succeeded and calls VoipModule.storeInitialEvents; change
answerIncomingCall(context: Context, callId: String) to return Boolean (true
when a VoiceConnection was found and .onAnswer() called, false when null path
taken), update finish(answerRequestSucceeded: Boolean) to call
answerIncomingCall(...) and combine its Boolean result with
answerRequestSucceeded before calling VoipModule.storeInitialEvents (i.e., only
call storeInitialEvents when both the network answer succeeded and
answerIncomingCall returned true), and keep the existing null-path cleanup
(disconnectIncomingCall, cancelById, ddpRegistry.stopClient,
VoipModule.storeAcceptFailureForJs) inside the now-returning-false branch;
locate these changes around the finish function and the answerIncomingCall and
VoiceConnectionService.getConnection usages.

🪄 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: f3ed8bc0-137b-40ac-8e39-179b460436c7

📥 Commits

Reviewing files that changed from the base of the PR and between d30c291 and 3d9a839.

📒 Files selected for processing (4)

• android/app/src/main/AndroidManifest.xml
• android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt
• android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
• app/lib/services/voip/MediaSessionInstance.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: ESLint and Test / run-eslint-and-test

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{js,jsx,ts,tsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line

Files:

• app/lib/services/voip/MediaSessionInstance.ts

**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

• app/lib/services/voip/MediaSessionInstance.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution

**/*.{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 numbers

Files:

• app/lib/services/voip/MediaSessionInstance.ts

app/lib/services/voip/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Files:

• app/lib/services/voip/MediaSessionInstance.ts

**/*.{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/lib/services/voip/MediaSessionInstance.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

📚 Learning: 2026-04-07T17:49:17.538Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Applied to files:

• app/lib/services/voip/MediaSessionInstance.ts
• android/app/src/main/AndroidManifest.xml
• android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
• android/app/src/main/java/chat/rocket/reactnative/voip/VoipCallService.kt

🔇 Additional comments (2)

android/app/src/main/AndroidManifest.xml (1)

150-155: LGTM — service is scoped correctly.

The service is non-exported and declares the microphone foreground service type used by VoipCallService.

app/lib/services/voip/MediaSessionInstance.ts (1)

167-167: LGTM — clearer native-accept fallback log.

The updated warning better identifies the post-accept missing-call path without changing behavior.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (1)

256-267: ⚠️ Potential issue | 🟠 Major

Short-circuit the accept flow when VoiceConnection is gone.

Line 326 stashes the accept-failed event, but answerIncomingCall returns Unit, so Line 262 still stores the normal initial payload afterward. Also, the foreground service started before the REST accept is not stopped on this null/failure path.

Proposed fix

                 ddpRegistry.stopClient(payload.callId)
                 if (answerRequestSucceeded) {
-                    answerIncomingCall(appCtx, payload.callId)
-                    VoipModule.storeInitialEvents(payload)
+                    if (answerIncomingCall(appCtx, payload.callId)) {
+                        VoipModule.storeInitialEvents(payload)
+                    } else {
+                        VoipCallService.stopService(appCtx)
+                    }
                 } else {
                     Log.d(TAG, "media-calls.answer failed for ${payload.callId}; opening app for JS recovery")
                     disconnectIncomingCall(payload.callId, false)
+                    VoipCallService.stopService(appCtx)
                     VoipModule.storeAcceptFailureForJs(payload)
                 }

-        private fun answerIncomingCall(context: Context, callId: String) {
+        private fun answerIncomingCall(context: Context, callId: String): Boolean {
             val connection = VoiceConnectionService.getConnection(callId)
-            when (connection) {
-                is VoiceConnection -> connection.onAnswer()
+            return when (connection) {
+                is VoiceConnection -> {
+                    connection.onAnswer()
+                    true
+                }
                 null -> {
                     // C3 fix: null return means Telecom connection is gone (system killed it).
                     // Notify JS so the user sees an error instead of a hanging UI.
                     Log.w(TAG, "No active VoiceConnection for accepted call: $callId — notifying JS of failure")
@@
                         createdAt = null,
                         voipAcceptFailed = true
                     ))
+                    false
                 }
-                else -> Log.d(TAG, "Non-VoiceConnection for accept, callId: $callId")
+                else -> {
+                    Log.d(TAG, "Non-VoiceConnection for accept, callId: $callId")
+                    false
+                }
             }
         }

Also applies to: 313-337

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`
around lines 256 - 267, In finish(answerRequestSucceeded: Boolean) short-circuit
the "accept" success path when the underlying VoiceConnection is missing or
answerIncomingCall failed: after calling answerIncomingCall(appCtx,
payload.callId) check that a valid VoiceConnection was returned (or that
answerIncomingCall indicates success) and if it's null/failed, stop the
foreground service started earlier, call disconnectIncomingCall(payload.callId,
false), call VoipModule.storeAcceptFailureForJs(payload) and return early so
VoipModule.storeInitialEvents(payload) is not executed; apply the same guard in
the equivalent block around the other accept path (lines ~313-337) to keep
behavior consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 256-267: In finish(answerRequestSucceeded: Boolean) short-circuit
the "accept" success path when the underlying VoiceConnection is missing or
answerIncomingCall failed: after calling answerIncomingCall(appCtx,
payload.callId) check that a valid VoiceConnection was returned (or that
answerIncomingCall indicates success) and if it's null/failed, stop the
foreground service started earlier, call disconnectIncomingCall(payload.callId,
false), call VoipModule.storeAcceptFailureForJs(payload) and return early so
VoipModule.storeInitialEvents(payload) is not executed; apply the same guard in
the equivalent block around the other accept path (lines ~313-337) to keep
behavior consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f606d151-75f7-4f49-b671-068dab769fb3

📥 Commits

Reviewing files that changed from the base of the PR and between 3d9a839 and d8116f7.

📒 Files selected for processing (2)

• android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
• app/lib/services/voip/MediaSessionInstance.ts

✅ Files skipped from review due to trivial changes (1)

• app/lib/services/voip/MediaSessionInstance.ts

📜 Review details

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

📚 Learning: 2026-04-07T17:49:17.538Z

Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/lib/services/voip/**/*.{ts,tsx} : Implement VoIP with WebRTC peer-to-peer audio calls in app/lib/services/voip/ using Zustand stores instead of Redux, with native CallKit (iOS) and Telecom (Android) integration; keep VoIP and VideoConf separate

Applied to files:

• android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt