feat(voip): migrate Android accept/reject from DDP to REST#7127
Open
diegolmello wants to merge 6 commits intofeat.voip-lib-newfrom
Open
feat(voip): migrate Android accept/reject from DDP to REST#7127diegolmello wants to merge 6 commits intofeat.voip-lib-newfrom
diegolmello wants to merge 6 commits intofeat.voip-lib-newfrom
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Member
Author
PRDPRD: Migrate VoIP Accept/Reject from DDP to REST (Android)Problem StatementThe native Android VoIP implementation opens a per-call DDP WebSocket connection solely to send two signals: accept and reject. This requires The same accept/reject signal can be sent via a single HTTP POST to SolutionReplace DDP-based accept/reject signaling from User Stories
ArchitectureOut of Scope
|
Member
Author
Progress
Slice 1 —
|
| User Story | Covered by |
|---|---|
| 1 — Reliable accept/reject signals | Slices 1–4 |
| 2 — Understandable VoipNotification handlers | Slices 2–5 |
| 3 — Testable without WebSocket | Slices 1, 2, 3 |
| 4 — Same Ejson auth mechanism | Slice 1 |
| 5 — Immediate response (no DDP handshake) | Slices 2, 3, 4 |
| 6 — Remove DDP from VoIP stack | Slice 5 (partial) |
| 7 — Independent from iOS | All slices |
Member
Author
File:
|
Contributor
WalkthroughAdds a new Kotlin REST client to POST /api/v1/media-calls.answer and refactors VoIP notification flows to use that REST call instead of DDP; also switches DDP WebSocket usage to a shared OkHttpClient instance. Changes
Sequence Diagram(s)sequenceDiagram
participant VN as VoipNotification
participant Settings as Settings.Secure
participant MCAR as MediaCallsAnswerRequest
participant Ejson as Ejson (Auth)
participant HTTP as OkHttp
participant API as Media Calls API
VN->>Settings: get ANDROID_ID (contractId)
VN->>MCAR: fetch(context, host, callId, contractId, answer, features, onResult)
MCAR->>Ejson: resolve x-user-id & x-auth-token
alt credentials missing
MCAR->>VN: onResult(false)
else credentials present
MCAR->>MCAR: build JSON body (callId, contractId, type, answer, supportedFeatures?)
MCAR->>HTTP: POST /api/v1/media-calls.answer with headers & body
HTTP->>API: send request
API-->>HTTP: HTTP response
HTTP->>MCAR: onResponse(status)
alt 2xx
MCAR->>VN: onResult(true)
else
MCAR->>VN: onResult(false)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ 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. Comment |
Member
Author
File:
|
diegolmello
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![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 (3)
android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (2)
153-161: Consider logging REST failures for decline action.The callback
{ _ -> }silently discards the result. While fire-and-forget semantics may be acceptable for decline (local UI proceeds regardless), logging failures would aid debugging when reject signals don't reach the server.Proposed fix
MediaCallsAnswerRequest.fetch( context = context, host = payload.host, callId = payload.callId, contractId = deviceId, answer = "reject", supportedFeatures = null - ) { _ -> } + ) { success -> + if (!success) { + Log.w(TAG, "REST reject failed for ${payload.callId}; local decline proceeds") + } + }🤖 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 153 - 161, The current decline call to MediaCallsAnswerRequest.fetch in VoipNotification.kt discards the result via `{ _ -> }`; update the callback to capture the result or error, and log failures using the app logger (include context such as payload.callId and deviceId) so rejected signals that fail to reach the server are recorded; modify the anonymous callback passed to MediaCallsAnswerRequest.fetch (the closure after answer="reject") to inspect the response/error and call the existing logging facility rather than ignoring it.
418-426: Same suggestion: log failures for busy-call rejection.Consistent with the decline path, logging REST failures here would help diagnose issues where busy-call rejections don't reach the server.
Proposed fix
MediaCallsAnswerRequest.fetch( context = context, host = payload.host, callId = payload.callId, contractId = deviceId, answer = "reject", supportedFeatures = null - ) { _ -> } + ) { success -> + if (!success) { + Log.w(TAG, "REST reject (busy) failed for ${payload.callId}") + } + }🤖 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 418 - 426, The busy-call rejection path currently calls MediaCallsAnswerRequest.fetch without logging failures; update the callback passed to MediaCallsAnswerRequest.fetch (in VoipNotification.kt where deviceId and payload.callId are used) to handle the response/error and call the same logger used on the decline path (e.g., process or repository logger) to record any non-success or exception details, including host, callId and the error message so REST failures are visible in logs. Ensure you preserve the existing parameters (context, host, callId, contractId=deviceId, answer="reject") and only add failure logging in the completion lambda.android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt (1)
82-95: Unusedcontextparameter.The
contextparameter is passed toexecute()but never used —Ejsonis instantiated without it. IfEjsonrelies on static/shared storage keyed by host, consider removing the parameter to avoid confusion. Otherwise, ifEjsonshould use the context, pass it to the constructor.🤖 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/MediaCallsAnswerRequest.kt` around lines 82 - 95, The execute function's context parameter is unused; either remove it from the execute signature and all callers, or thread it into Ejson by constructing Ejson with the context (e.g., Ejson(context).apply { host = host }) so the instance can access Android resources; update the execute method signature and every place that calls MediaCallsAnswerRequest.execute, or change the Ejson instantiation inside execute to accept context, and ensure imports and constructors are updated accordingly.
🤖 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/MediaCallsAnswerRequest.kt`:
- Around line 110-123: Replace per-request OkHttpClient instantiation with a
shared/reused client (e.g., a companion object or singleton HttpClient) instead
of calling OkHttpClient() inside MediaCallsAnswerRequest; also ensure the
okhttp3.Response is closed in onResponse to avoid leaks (use response.close() or
response.body?.close() / response.use { ... } before returning) and continue to
pass the success boolean to onResult; locate these changes around the
newCall(request).enqueue callback and the onFailure/onResponse overrides
referenced in the diff.
---
Nitpick comments:
In
`@android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt`:
- Around line 82-95: The execute function's context parameter is unused; either
remove it from the execute signature and all callers, or thread it into Ejson by
constructing Ejson with the context (e.g., Ejson(context).apply { host = host })
so the instance can access Android resources; update the execute method
signature and every place that calls MediaCallsAnswerRequest.execute, or change
the Ejson instantiation inside execute to accept context, and ensure imports and
constructors are updated accordingly.
In `@android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt`:
- Around line 153-161: The current decline call to MediaCallsAnswerRequest.fetch
in VoipNotification.kt discards the result via `{ _ -> }`; update the callback
to capture the result or error, and log failures using the app logger (include
context such as payload.callId and deviceId) so rejected signals that fail to
reach the server are recorded; modify the anonymous callback passed to
MediaCallsAnswerRequest.fetch (the closure after answer="reject") to inspect the
response/error and call the existing logging facility rather than ignoring it.
- Around line 418-426: The busy-call rejection path currently calls
MediaCallsAnswerRequest.fetch without logging failures; update the callback
passed to MediaCallsAnswerRequest.fetch (in VoipNotification.kt where deviceId
and payload.callId are used) to handle the response/error and call the same
logger used on the decline path (e.g., process or repository logger) to record
any non-success or exception details, including host, callId and the error
message so REST failures are visible in logs. Ensure you preserve the existing
parameters (context, host, callId, contractId=deviceId, answer="reject") and
only add failure logging in the completion lambda.
🪄 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: ee27a59b-e07a-4dfa-8c3d-34dd29c7989a
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
📜 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
🧠 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
🔇 Additional comments (3)
android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt (1)
1-53: Clean implementation of the REST client.The overall structure is well-designed: good KDoc documentation, clear separation between the factory method and instance execution, and proper credential resolution pattern matching the existing
ReplyBroadcastapproach.android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt (2)
276-286: Accept path handles REST result correctly.The accept flow properly integrates with the existing
finish()callback pattern, preserving the 10-second timeout and JS recovery path on failure. TheddpRegistry.stopClientcall infinish()appropriately cleans up any DDP listener started bystartListeningForCallEnd.
395-403: Retained helper for DDP subscription flush.This function is correctly preserved for use in
startListeningForCallEnd— it flushes any queued DDP method calls after login completes. Since call-end detection remains on DDP (deferred slice), this is still needed.
android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt
Outdated
Show resolved
Hide resolved
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
force-pushed
the
refactor.ddp-android-2
branch
from
a611d73 to
38ab136
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt (1)
48-48: Protectconnect()against duplicate calls.Line 48 overwrites the current
webSocketreference. If the sameDDPClientreconnects without a priordisconnect(), the old listener can keep delivering messages into the same callback state. Either reject re-entry up front or tag listener callbacks with a connection generation before swapping the socket.🤖 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/DDPClient.kt` at line 48, The connect() method currently assigns a new WebSocket to the webSocket field unconditionally which can overwrite an active socket and keep old WebSocketListener callbacks delivering messages into the new state; either prevent re-entrancy by checking webSocket (or a boolean isConnected) and returning/erroring if already connected, or implement a connection generation/tag (e.g., connectionId or generation counter stored on DDPClient) and have the anonymous WebSocketListener capture that generation and ignore callbacks if it does not match before swapping webSocket; update disconnect() to clear/reset the flag or advance the generation so stale listeners are ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt`:
- Line 48: The connect() method currently assigns a new WebSocket to the
webSocket field unconditionally which can overwrite an active socket and keep
old WebSocketListener callbacks delivering messages into the new state; either
prevent re-entrancy by checking webSocket (or a boolean isConnected) and
returning/erroring if already connected, or implement a connection
generation/tag (e.g., connectionId or generation counter stored on DDPClient)
and have the anonymous WebSocketListener capture that generation and ignore
callbacks if it does not match before swapping webSocket; update disconnect() to
clear/reset the flag or advance the generation so stale listeners are ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccc4ee55-682e-4ba5-8fd4-2faba85f9f0e
📒 Files selected for processing (2)
android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (1)
📓 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
🔇 Additional comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt (1)
24-30: This ownership shift looks good.Making the transport instance shared removes the need for one
DDPClientto tear it down for every other caller.
Adds android equivalent of ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift.
POST /api/v1/media-calls.answer with { callId, contractId, type, answer, supportedFeatures }.
Auth via Ejson (x-user-id / x-auth-token) — same pattern as ReplyBroadcast.
Unblocks Slices 2, 3, 4.
…swerRequest
Address CodeRabbit resource leak warning: reuse the OkHttpClient singleton
instead of creating a new instance per request, and wrap the Response in
response.use {} to ensure the body is closed after use.
diegolmello
force-pushed
the
refactor.ddp-android-2
branch
from
38ab136 to
b85201a
Compare
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
diegolmello
deployed
to
experimental_android_build
GitHub Actions
Active
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt (1)
47-49: Constrainanswerand gatesupportedFeaturesby answer type.
answeris free-form on Line 47, andsupportedFeaturesis serialized whenever non-null on Line 75. This allows invalid payloads if a caller passes unexpected combinations.Proposed hardening diff
class MediaCallsAnswerRequest( private val callId: String, private val contractId: String, - private val answer: String, + private val answer: String, private val supportedFeatures: List<String>? = null ) { companion object { @@ + enum class Answer(val value: String) { ACCEPT("accept"), REJECT("reject") } + `@JvmStatic` fun fetch( context: android.content.Context, host: String, callId: String, contractId: String, - answer: String, + answer: Answer, supportedFeatures: List<String>? = null, onResult: (Boolean) -> Unit ) { - val request = MediaCallsAnswerRequest(callId, contractId, answer, supportedFeatures) + val request = MediaCallsAnswerRequest(callId, contractId, answer.value, supportedFeatures) request.execute(context, host, onResult) } } @@ - supportedFeatures?.let { features -> - val arr = JSONArray() - features.forEach { arr.put(it) } - json.put("supportedFeatures", arr) - } + if (answer == "accept") { + supportedFeatures?.let { features -> + val arr = JSONArray() + features.forEach { arr.put(it) } + json.put("supportedFeatures", arr) + } + } return json }Also applies to: 75-79
🤖 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/MediaCallsAnswerRequest.kt`:
- Around line 111-123: The OkHttp callbacks in MediaCallsAnswerRequest (methods
onFailure and onResponse) call onResult directly on a background thread; change
both calls to post to the main thread using Handler(Looper.getMainLooper()).post
{ ... } so that onResult (and downstream VoipNotification.finish) runs on the UI
thread; update the onFailure block and the onResponse block to wrap their
onResult(...) invocations in Handler(Looper.getMainLooper()).post {
onResult(...) }.
🪄 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: 469fe359-318f-486a-b399-39328e9bcaf3
📒 Files selected for processing (3)
android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.ktandroid/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- android/app/src/main/java/chat/rocket/reactnative/voip/DDPClient.kt
- android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
📜 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (1)
📓 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
🔇 Additional comments (2)
android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt (2)
92-96: Credential guard looks solid.Failing fast when
x-user-id/x-auth-tokenare missing (Line 92) is the right behavior for this path and avoids pointless network attempts.
117-123: Great response lifecycle handling.Using
response.use { ... }on Line 118 correctly closes the response and prevents resource leaks.
Comment on lines
+111
to
+123
| httpClient.newCall(request).enqueue(object : Callback { | ||
| override fun onFailure(call: Call, e: IOException) { | ||
| Log.e(TAG, "MediaCallsAnswerRequest failed for callId=$callId: ${e.message}") | ||
| onResult(false) | ||
| } | ||
|
|
||
| override fun onResponse(call: Call, response: okhttp3.Response) { | ||
| response.use { | ||
| val code = it.code | ||
| val success = code in 200..299 | ||
| Log.d(TAG, "MediaCallsAnswerRequest response for callId=$callId: code=$code success=$success") | ||
| onResult(success) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect callsites and callback bodies of MediaCallsAnswerRequest.fetch usage.
rg -nC6 --type=kt '\bMediaCallsAnswerRequest\.fetch\s*\('
# Check whether those callback paths explicitly switch to main thread.
rg -nC4 --type=kt 'Looper\.getMainLooper|Handler\(|runOnUiThread|post\s*\{'Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 104
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the MediaCallsAnswerRequest file
fd -e kt MediaCallsAnswerRequest
# Search for calls to MediaCallsAnswerRequest.fetch (without type restrictions)
rg -n 'MediaCallsAnswerRequest\.fetch' --glob='*.kt' --glob='*.java'
# Also search for where onResult is being called with from context
rg -n 'onResult\s*\(' --glob='*.kt' --glob='*.java' -C3 | head -100Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 3174
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get context around the MediaCallsAnswerRequest.fetch calls in VoipNotification.kt
echo "=== Callsite 1 (line 154) ==="
sed -n '150,170p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
echo "=== Callsite 2 (line 277) ==="
sed -n '273,293p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
echo "=== Callsite 3 (line 419) ==="
sed -n '415,435p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
# Also get the fetch method signature in MediaCallsAnswerRequest.kt
echo "=== MediaCallsAnswerRequest.fetch signature ==="
sed -n '70,105p' android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.ktRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 4049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check VoipNotification class declaration
head -20 android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
# Check the finish method to see if it's from Activity
rg -n 'class VoipNotification' --type=java --type=kt -A5
# Check if there's a custom OkHttp dispatcher in MediaCallsAnswerRequest
rg -n 'Dispatcher|httpClient|OkHttpClient' android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt -B2 -A2Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the class declaration
grep -n "^class VoipNotification" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
# Check finish method context - what class it's from
sed -n '270,290p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
# Check for httpClient configuration in MediaCallsAnswerRequest
grep -n "httpClient\|OkHttpClient\|Dispatcher" android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.kt
# Also check if there's any Handler or postToMainThread usage
grep -n "Handler\|runOnUiThread\|post\|post(" android/app/src/main/java/chat/rocket/reactnative/voip/MediaCallsAnswerRequest.ktRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the finish() method in VoipNotification
grep -n "fun finish" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt -A15Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find answerIncomingCall and disconnectIncomingCall methods
echo "=== answerIncomingCall ==="
grep -n "fun answerIncomingCall" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt -A10
echo "=== disconnectIncomingCall ==="
grep -n "fun disconnectIncomingCall" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt -A10
# Check the full finish method to ensure we saw all of it
echo "=== Full finish method ==="
sed -n '246,265p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.ktRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 2455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if finish() or any of its callees already handle main thread posting
echo "=== Check for main thread handling in finish context ==="
sed -n '245,270p' android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt
# Check if VoiceConnection operations are thread-aware
rg -n "VoiceConnection\|storeInitialEvents\|storeAcceptFailureForJs" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt -B2 -A2 | head -30
# Check if there's a Handler defined in VoipNotification for main thread posting
grep -n "Handler\|Looper" android/app/src/main/java/chat/rocket/reactnative/voip/VoipNotification.kt | head -10Repository: RocketChat/Rocket.Chat.ReactNative
Length of output: 1382
Post onResult to main thread from OkHttp callback boundary.
OkHttp invokes callbacks on a background dispatcher thread by default. The current implementation calls finish(success) directly from the callback (line 277 in VoipNotification.kt), which performs thread-sensitive operations: LocalBroadcastManager.sendBroadcast(), Activity launching, and VoiceConnection state changes. This can cause crashes or undefined behavior. Wrap the onResult invocation with Handler(Looper.getMainLooper()).post { } to ensure these operations execute safely on the main thread.
🤖 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/MediaCallsAnswerRequest.kt`
around lines 111 - 123, The OkHttp callbacks in MediaCallsAnswerRequest (methods
onFailure and onResponse) call onResult directly on a background thread; change
both calls to post to the main thread using Handler(Looper.getMainLooper()).post
{ ... } so that onResult (and downstream VoipNotification.finish) runs on the UI
thread; update the onFailure block and the onResponse block to wrap their
onResult(...) invocations in Handler(Looper.getMainLooper()).post {
onResult(...) }.
|
Android Build Available Rocket.Chat 4.72.0.108528 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNTZJOI7OEsicACr_uxIS0L_nhUyR3FFpF-OEZJKVwDfWyK9uoaAbmSRJ_I-xyrttmzbgyYyg4HUtopascg3 |
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
1 participant
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.
Proposed changes
Migrate the VoIP accept/reject signal path on Android from the DDP (WebSocket) transport to a synchronous REST call (
POST /api/v1/media-calls.answer), matching the pattern already implemented on iOS.Summary
MediaCallsAnswerRequest.ktREST client9692a23c0handleAcceptAction→ REST669c19105handleDeclineAction→ REST01d119185rejectBusyCall→ RESTd0dc50874VoipNotification8d543e45aDeferred
startListeningForCallEndoff DDP (deferred to separate PRD)DDPClient.kt/VoipPerCallDdpRegistry.kt(blocked by Slice 6)Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-67
How to test or reproduce
MediaCallsAnswerRequest responserejectBusyCallsends REST rejectScreenshots
N/A — no UI changes.
Types of changes
Checklist
Summary by CodeRabbit
Improvements
Stability