feat(voip): migrate iOS accept/reject from DDP to REST#7124
Open
diegolmello wants to merge 10 commits intofeat.voip-lib-newfrom
Open
feat(voip): migrate iOS accept/reject from DDP to REST#7124diegolmello wants to merge 10 commits intofeat.voip-lib-newfrom
diegolmello wants to merge 10 commits intofeat.voip-lib-newfrom
Conversation
Accept/reject now go through REST (MediaCallsAnswerRequest) in handleNativeAccept and reject(). The DDP-based buildMediaCallAnswerParams and its VoipMediaCallAnswerKind enum are no longer referenced anywhere.
Member
Author
PRDProblem StatementThe native iOS VoIP implementation opens a per-call WebSocket (DDP) connection solely to send two signals: accept and reject. This requires a full DDP client with connect/login/subscribe/callback-queue logic that is significantly more complex than the actual business logic. The same accept/reject signal can be sent via a single HTTP POST using the server's existing The DDP complexity also makes the code hard to reason about, test, and maintain — particularly the SolutionReplace the DDP-based accept/reject signaling from iOS native ( The DDP listener for call-end detection (hangup from remote, accepted-on-another-device) is not migrated — it requires a persistent WebSocket subscription and is a separate problem. User Stories
ModulesNew:
Modify:
Delete: AuthThe Payload shape{
"callId": "...",
"contractId": "DeviceUID",
"answer": "accept|reject",
"supportedFeatures": ["audio"] // only on accept
}ArchitectureOut of Scope
|
Member
Author
Implementation ProgressSlice 1 —
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMedia call accept/reject signaling is migrated from DDP-based to REST API-based communication. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
![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: 2
🧹 Nitpick comments (2)
ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift (1)
28-38: Silent failure inbody()could mask issues.Using
try?means ifJSONSerialization.data(withJSONObject:)fails, the request body is silentlynil. While failure is unlikely for this simple dictionary, consider logging or handling the error for debuggability during development.That said, for these basic types (
String,[String]?), serialization failure is practically impossible, so this is acceptable as-is.💡 Optional: Add debug logging for serialization failure
func body() -> Data? { var dict: [String: Any] = [ "callId": callId, "contractId": contractId, "answer": answer ] if let features = supportedFeatures { dict["supportedFeatures"] = features } - return try? JSONSerialization.data(withJSONObject: dict) + do { + return try JSONSerialization.data(withJSONObject: dict) + } catch { + `#if` DEBUG + print("[MediaCallsAnswerRequest] JSON serialization failed: \(error)") + `#endif` + return nil + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift` around lines 28 - 38, The body() method currently swallows JSONSerialization errors with try?, which can hide rare failures; update MediaCallsAnswerRequest.body() to use do-catch around JSONSerialization.data(withJSONObject:) (or propagate the thrown error) and in the catch log the error via the existing logging facility (or return nil only after logging) so serialization failures are visible during debugging while preserving the same return semantics for production.ios/Libraries/VoipService.swift (1)
519-521: Consider logging rejected call failures for observability.The completion handler discards the result entirely. While reject doesn't need to affect UI, logging failures would help diagnose issues where callers see "missed call" notifications despite the callee having rejected.
📊 Optional: Add debug logging
)) { _ in + `#if` DEBUG + print("[\(TAG)] Reject request completed for call \(payload.callId)") + `#endif` self.stopDDPClientInternal(callId: payload.callId) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Libraries/VoipService.swift` around lines 519 - 521, The completion handler that currently just calls self.stopDDPClientInternal(callId: payload.callId) is dropping the reject result; update that closure to inspect the reject call result and log failures (including payload.callId and error details) for observability—use the app's existing logger (or NSLog/os_log if none) to emit a debug/error message when the reject operation fails before calling stopDDPClientInternal(callId:).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/Libraries/VoipService.swift`:
- Around line 471-485: API(server: payload.host) can be nil and currently the
fetch is skipped leaving finishAccept uncalled; update the call site to unwrap
the API(server: payload.host) result (or guard-let it) before calling fetch, and
if it is nil immediately call finishAccept(false) (optionally log the invalid
host and ensure the finishAccept call is dispatched to the main queue like the
existing completion path). Specifically, handle the nil case around the
API(server:) invocation used with fetch and MediaCallsAnswerRequest so every
execution path invokes finishAccept.
- Around line 513-522: The reject function doesn't handle API(server:) == nil so
no reject is sent and stopDDPClientInternal(callId:) may never be called; update
reject to handle the nil case by ensuring stopDDPClientInternal(callId:
payload.callId) is always invoked and, when API(server:) returns non-nil, still
send the MediaCallsAnswerRequest(callId:contractId:answer:supportedFeatures:) as
before; reference the reject(payload: VoipPayload) method, the API(server:)
initializer, MediaCallsAnswerRequest, and stopDDPClientInternal to locate and
implement the nil-guard and guaranteed cleanup.
---
Nitpick comments:
In `@ios/Libraries/VoipService.swift`:
- Around line 519-521: The completion handler that currently just calls
self.stopDDPClientInternal(callId: payload.callId) is dropping the reject
result; update that closure to inspect the reject call result and log failures
(including payload.callId and error details) for observability—use the app's
existing logger (or NSLog/os_log if none) to emit a debug/error message when the
reject operation fails before calling stopDDPClientInternal(callId:).
In `@ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift`:
- Around line 28-38: The body() method currently swallows JSONSerialization
errors with try?, which can hide rare failures; update
MediaCallsAnswerRequest.body() to use do-catch around
JSONSerialization.data(withJSONObject:) (or propagate the thrown error) and in
the catch log the error via the existing logging facility (or return nil only
after logging) so serialization failures are visible during debugging while
preserving the same return semantics for production.
🪄 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: 3f7f3778-f428-4684-aed7-c09ecccdecd8
📒 Files selected for processing (2)
ios/Libraries/VoipService.swiftios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift
📜 Review details
🧰 Additional context used
🧠 Learnings (3)
📓 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:
ios/Libraries/VoipService.swift
📚 Learning: 2026-03-05T06:06:19.755Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:19.755Z
Learning: In the Rocket.Chat React Native iOS app, `WCSession` (WatchConnectivity) is activated with its delegate in `ios/RocketChat Watch App/Loaders/WatchSession.swift`, not in `RCTWatchModule.mm`. Since `WCSession.defaultSession` is a singleton, activating it once in `WatchSession.swift` is sufficient; `RCTWatchModule.mm` does not need to re-activate or re-set the delegate.
Applied to files:
ios/Libraries/VoipService.swift
🔇 Additional comments (3)
ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift (1)
10-43: LGTM! Clean REST request/response implementation.The structure correctly implements the
Requestprotocol with proper HTTP method, path, and body serialization. The optionalsupportedFeatureshandling is appropriately conditional.ios/Libraries/VoipService.swift (2)
4-4: LGTM!Import added correctly for accessing
MediaCallsAnswerRequestfrom the RocketChat module.
505-507: LGTM! Clean migration to REST-based reject.The reject paths in both
rejectBusyCallandhandleObservedCallChangednow correctly route through the newreject(payload:)method, which uses the REST endpoint instead of DDP.Also applies to: 587-587
Handle the case when API(server:) returns nil by: - accept: call finishAccept(false) on the main queue with a DEBUG log - reject: call stopDDPClientInternal to clean up the DDP client with a DEBUG log Without this, a nil API silently skipped the REST call, leaving CallKit in an indeterminate state (accept) or DDP client leaking (reject).
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
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
When the user ended an incoming call before it connected, handleObservedCallChanged sent the REST reject but never notified CallKit to remove the call from the system Recents UI. Also deduplicate MediaCallsAnswerResponse with MessageResponse.
diegolmello
requested a deployment
to
approve_e2e_testing
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
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
When the user ended an incoming call before it connected, handleObservedCallChanged sent the REST reject but never notified CallKit to remove the call from the system Recents UI. Also deduplicate MediaCallsAnswerResponse with MessageResponse, and remove unnecessary import RocketChat that was causing module resolution failures in the Libraries target.
…ReactNative into refactor.ddp-ios
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/RocketChatRN.xcodeproj/project.pbxproj`:
- Line 370: MediaCallsAnswerRequest.swift currently has only one PBXBuildFile
entry so it's only compiled into the RocketChatRN target; add a second
PBXBuildFile entry (with a new unique PBX identifier) referencing the same
fileRef (MediaCallsAnswerRequest.swift) and include that new PBXBuildFile in the
Rocket.Chat target's Sources list to mirror the dual-target pattern used by
other shared Swift files (e.g., VoipService.swift), ensuring both RocketChatRN
and Rocket.Chat build the file.
🪄 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: f5e0215b-90b1-4f66-8b91-8ed591bfc8e6
⛔ Files ignored due to path filters (1)
ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
ios/Libraries/VoipService.swiftios/RocketChatRN.xcodeproj/project.pbxprojios/Shared/RocketChat/API/Requests/MediaCallsAnswerRequest.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/Libraries/VoipService.swift
📜 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: format
🧰 Additional context used
🧠 Learnings (3)
📓 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:
ios/RocketChatRN.xcodeproj/project.pbxproj
📚 Learning: 2026-03-05T06:06:19.755Z
Learnt from: divyanshu-patil
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6957
File: ios/RCTWatchModule.mm:19-24
Timestamp: 2026-03-05T06:06:19.755Z
Learning: In the Rocket.Chat React Native iOS app, `WCSession` (WatchConnectivity) is activated with its delegate in `ios/RocketChat Watch App/Loaders/WatchSession.swift`, not in `RCTWatchModule.mm`. Since `WCSession.defaultSession` is a singleton, activating it once in `WatchSession.swift` is sufficient; `RCTWatchModule.mm` does not need to re-activate or re-set the delegate.
Applied to files:
ios/RocketChatRN.xcodeproj/project.pbxproj
🔇 Additional comments (1)
ios/Shared/RocketChat/API/Requests/MediaCallsAnswerRequest.swift (1)
10-41: Nice extraction of the answer REST payload.Keeping this request isolated here makes the accept/reject migration much easier to follow, and the conditional
supportedFeaturesserialization matches the two call paths cleanly.
…RocketChat target Deleted the duplicate copy at Shared/RocketChat/API/ and kept the one at Shared/RocketChat/API/Requests/ to match SendMessage.swift layout. Added PBXBuildFile entry for the RocketChat target so VoipService.swift can reference MediaCallsAnswerRequest when compiled into that target.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ios/RocketChatRN.xcodeproj/project.pbxproj (1)
1144-1151: Remove the orphanedDeleted Rogue API Group.This adds a second top-level
RocketChatgroup that only contains an empty placeholder group. It won’t affect the build, but it will make the navigator misleading.Cleanup diff
- 543368E8E0F7A6A03AD7C8F9 /* RocketChat */ = { - isa = PBXGroup; - children = ( - 9327C58E3506F20CD4FCE7E0 /* API */, - ); - name = RocketChat; - sourceTree = "<group>"; - }; ... - 543368E8E0F7A6A03AD7C8F9 /* RocketChat */, ... - 9327C58E3506F20CD4FCE7E0 /* API */ = { - isa = PBXGroup; - children = ( - ); - name = "Deleted Rogue API Group"; - sourceTree = "<group>"; - };Also applies to: 1214-1214, 1233-1239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/RocketChatRN.xcodeproj/project.pbxproj` around lines 1144 - 1151, Remove the orphaned duplicate top-level PBXGroup block (the PBXGroup with GUID 543368E8E0F7A6A03AD7C8F9 named "RocketChat" that only contains the child GUID 9327C58E3506F20CD4FCE7E0 /* API */) and any referenced empty child group entries (e.g., 9327C58E3506F20CD4FCE7E0) from the project.pbxproj; also remove any references to these GUIDs from other PBXGroup children arrays so only the real RocketChat group remains (check the other duplicate blocks mentioned in the review and delete the redundant PBXGroup and its child placeholders).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/RocketChatRN.xcodeproj/project.pbxproj`:
- Around line 1144-1151: Remove the orphaned duplicate top-level PBXGroup block
(the PBXGroup with GUID 543368E8E0F7A6A03AD7C8F9 named "RocketChat" that only
contains the child GUID 9327C58E3506F20CD4FCE7E0 /* API */) and any referenced
empty child group entries (e.g., 9327C58E3506F20CD4FCE7E0) from the
project.pbxproj; also remove any references to these GUIDs from other PBXGroup
children arrays so only the real RocketChat group remains (check the other
duplicate blocks mentioned in the review and delete the redundant PBXGroup and
its child placeholders).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 186b3c92-f02d-441c-8b04-957d42980136
📒 Files selected for processing (1)
ios/RocketChatRN.xcodeproj/project.pbxproj
📜 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: format
🧰 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:
ios/RocketChatRN.xcodeproj/project.pbxproj
🔇 Additional comments (1)
ios/RocketChatRN.xcodeproj/project.pbxproj (1)
370-372:MediaCallsAnswerRequest.swiftis wired into the shared native targets correctly.The file reference, group placement, and
Sourcesmemberships line up cleanly forRocketChatRN,Rocket.Chat, andNotificationService.Also applies to: 660-660, 893-893, 2157-2157, 2380-2380, 2439-2439
Conflict in ios/RocketChatRN.xcodeproj/project.pbxproj: both branches modified Pods-related entries with different UUIDs (different pod install runs). Resolution: take feat.voip-lib-new as base, inject MediaCallsAnswerRequest entries from refactor.ddp-ios, then re-run pod install to regenerate consistent UUIDs. - Added MediaCallsAnswerRequest.swift PBXBuildFile entries for RocketChatRN, NotificationService, and Rocket.Chat targets - Added PBXFileReference for MediaCallsAnswerRequest.swift - Added PBXGroup children entry for the file - Added PBXSourcesBuildPhase entries for all three targets
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 VoIP
accept()andreject()on iOS from DDP (WebSocket) signaling to REST calls against the existingPOST /api/v1/media-calls.answerendpoint. This eliminates the per-call WebSocket lifecycle (connect → DDP connect → login → subscribe → send) from the accept/reject path, significantly simplifying the native iOS VoIP layer.What changed:
MediaCallsAnswerRequestAPI request struct (ios/Shared/RocketChat/API/MediaCallsAnswerRequest.swift)VoipService.accept(): replaced DDPclient.callMethod/queueMethodCallwithAPI.fetch(request:)VoipService.reject(): replaced DDP with REST (renamed fromsendRejectSignal)buildMediaCallAnswerParams()andVoipMediaCallAnswerKindenumDDPClient/VoipPerCallDdpRegistrykept — still used bystartListeningForCallEnd(call-end DDP listener)What did NOT change:
startListeningForCallEndDDP subscription (separate problem — persistent WebSocket needed for real-time call-end events)Issue(s)
https://rocketchat.atlassian.net/browse/VMUX-67
How to test or reproduce
POST /api/v1/media-calls.answeris called withanswer: "accept"POST /api/v1/media-calls.answeris called withanswer: "reject"stream-notify-user)Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit