feat: callkit/telecom integration #2028
Conversation
…lkit-telecom-integration # Conflicts: # sample-apps/react-native/dogfood/ios/Podfile.lock # yarn.lock
…/GetStream/stream-video-js into feat/callkit-telecom-integration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/Call.ts (1)
658-669: Preserve the pre-leave ringing flag for callManager.stop.
this.ringingis set tofalsejust beforestop(), soisRingingTypeCallalways becomesfalse. Capture the original value or move the reset after the stop call.🐛 Proposed fix
- this.unifiedSessionId = undefined; - this.ringingSubject.next(false); + this.unifiedSessionId = undefined; + const wasRingingTypeCall = this.ringing; + this.ringingSubject.next(false); this.cancelAutoDrop(); this.clientStore.unregisterCall(this); - globalThis.streamRNVideoSDK?.callManager.stop({ - isRingingTypeCall: this.ringing, - }); + globalThis.streamRNVideoSDK?.callManager.stop({ + isRingingTypeCall: wasRingingTypeCall, + });
🤖 Fix all issues with AI agents
In `@packages/react-native-callingx/package.json`:
- Around line 5-12: package.json currently only exposes an ESM module and types
but sets "main" to an ESM path; add a CommonJS build target and update
package.json so CommonJS consumers work: produce a CommonJS output at
dist/commonjs (in your build config) and change "main" to
"./dist/commonjs/index.js", and update the "exports" map to include a commonjs
entry (e.g., add a "require" or "commonjs" key pointing to
"./dist/commonjs/index.js") alongside the existing "source"/"types"/"default"
entries to mirror the react-native-sdk layout.
In `@packages/react-native-sdk/ios/StreamInCallManager.swift`:
- Around line 145-147: The two calls to session.setPreferredSampleRate and
session.setPreferredIOBufferDuration should be wrapped in their own try-catch
blocks so a failure in either doesn't skip ADM restoration; after adm.reset()
ensure each of session.setPreferredSampleRate(rtcConfig.sampleRate) and
session.setPreferredIOBufferDuration(rtcConfig.ioBufferDuration) is attempted
inside its own try/catch that logs the error but continues, and then always call
adm.setRecording(...) and adm.setPlayout(...) to restore the audio device module
(refer to adm.reset(), adm.setRecording(), adm.setPlayout(),
setPreferredSampleRate, setPreferredIOBufferDuration to locate the changes).
In `@packages/react-native-sdk/src/utils/internal/registerSDKGlobals.ts`:
- Around line 37-56: The StreamInCallManagerNativeModule calls in the setup,
start, and stop handlers (setup, start, stop in registerSDKGlobals.ts) need
try/catch guards to prevent unhandled native-module rejections; for each
handler, keep the existing shouldBypassForCallKit({ isRingingTypeCall })
early-return, then wrap
StreamInCallManagerNativeModule.setDefaultAudioDeviceEndpointType/defaultDevice,
StreamInCallManagerNativeModule.setup(),
StreamInCallManagerNativeModule.start(), and
StreamInCallManagerNativeModule.stop() in try/catch blocks and, inside the
catch, emit an optional debug log only when __DEV__ is true (include the caught
error in the message).
In `@packages/react-native-sdk/src/utils/push/setupCallingExpEvents.ts`:
- Around line 18-43: Update the top-of-file JSDoc and inline NOTE in
setupCallingExpEvents to accurately reference the CallingX library (not
CallKeep) and remove "hook" wording; edit the opening comment above
setupCallingExpEvents to say something like "Set up CallingX event listeners for
incoming/ending calls" and correct typos and grammar in the NOTE that explains
delayed events from callingx.getInitialEvents(); ensure referenced symbols
(setupCallingExpEvents, getCallingxLib, callingx, getInitialEvents,
onAcceptCall, onEndCall) remain unchanged and only the explanatory text is
clarified for maintainers.
In `@sample-apps/react/react-dogfood/components/MeetingUI.tsx`:
- Around line 75-80: The partialUpdateUser call is including email: undefined in
the payload; update the code that builds the "set" object before calling
chatClient.partialUpdateUser (the block using options.displayName /
getRandomName / sanitizeUserId to derive name and id) to only add the email
property when chatClient?.user?.email is defined—construct a set object with
name and conditionally add email if non-undefined, then pass that set to
partialUpdateUser (keeping the existing id and the catch for errors).
🧹 Nitpick comments (2)
packages/client/src/types.ts (1)
348-355: Consider exporting the parameter types for API completeness.These types define the method signatures of the exported
StreamRNVideoSDKGlobalsinterface. Consumers implementing a call manager conforming to this interface would benefit from being able to reference these types directly.Suggested change
-type StreamRNVideoSDKCallManagerRingingParams = { +export type StreamRNVideoSDKCallManagerRingingParams = { isRingingTypeCall: boolean; }; -type StreamRNVideoSDKCallManagerSetupParams = +export type StreamRNVideoSDKCallManagerSetupParams = StreamRNVideoSDKCallManagerRingingParams & { defaultDevice: AudioSettingsRequestDefaultDeviceEnum; };As per coding guidelines: "Make public API surfaces explicit with TypeScript types and interfaces".
packages/client/src/devices/SpeakerManager.ts (1)
24-58: Well-structured React Native speaker device configuration.The priority-based logic is clearly documented and the change detection prevents unnecessary native bridge calls. The scoped logger usage aligns with project conventions. All dependencies (
call.ringing,callManager.setup) are properly available and correctly typed.Consider adding a warning log when the SDK global is unavailable in React Native environments (i.e., when
isReactNative()is true butglobalThis.streamRNVideoSDKis undefined), to aid in debugging configuration issues.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-native-sdk/package.json (1)
61-78: Breaking change requires proper version bump and CHANGELOG documentation.Removing
react-native-callkeepandreact-native-voip-push-notificationpeer dependencies is a breaking change, but it's not properly documented:
- Version bump: This change should trigger a MINOR version bump (e.g., 1.29.0) at minimum, not a PATCH version. Removing peer dependencies is a breaking change per semver.
- CHANGELOG: Add a "BREAKING CHANGES" section to the version entry documenting the removal and directing users to
@stream-io/react-native-callingxas the replacement.- Migration guide: Add setup and migration instructions to the README or a dedicated migration document covering the transition from the legacy libraries to the new
react-native-callingxpackage.- Update CLAUDE.md: Remove or update references to "CallKeep integration" since that dependency is being removed in favor of the new callingx package.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/react-native-sdk/src/hooks/useAndroidKeepCallAliveEffect.ts`:
- Around line 89-91: The assignment keepCallAliveCallRef.current = call is a
render-phase side effect; move it into an effect by creating a useEffect that
depends on the call (from useCall) and sets keepCallAliveCallRef.current there,
ensuring cleanup if needed; update any logic that reads activeCallCid to derive
it from call inside the render or from the ref inside effects as appropriate so
no mutation occurs during render (refer to keepCallAliveCallRef, useCall, and
activeCallCid).
- Around line 177-189: The promise returned by
notifeeLib.default.getDisplayedNotifications() in
useAndroidKeepCallAliveEffect.ts lacks error handling; update the chain on the
notifeeLib check (referencing notifeeLib, getDisplayedNotifications,
activeCallCid, stopForegroundService) to append a .catch(...) that logs or
handles the error (e.g., via console.error or your logger) so any rejection is
handled and does not produce an unhandled promise rejection.
- Around line 55-59: The code accesses nested properties without guarding for
undefined (videoConfig.foregroundService.android and videoConfig.push.android),
which can throw; update the retrievals in useAndroidKeepCallAliveEffect
(references: videoConfig, foregroundServiceConfig, notificationTexts, channel,
smallIconName) to use optional chaining and sensible defaults or null checks
(e.g., access foregroundServiceConfig?.android?.notificationTexts and
foregroundServiceConfig?.android?.channel and
videoConfig.push?.android?.smallIcon) and handle the case where these values are
undefined before using them.
Co-authored-by: Artem Grintsevich <greenfrvr@gmail.com>
# Conflicts: # sample-apps/react-native/dogfood/package.json
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/Call.ts (1)
888-958:⚠️ Potential issue | 🟠 MajorSerialize join() to prevent concurrent join races
callingStateis checked before it flips toJOINING(insidedoJoin()), so two concurrentjoin()calls can still slip through and execute in parallel. Wrappingjoin()withwithoutConcurrencyensures a single in-flight join/leave operation and prevents duplicatecallingXregistrations and retry loops.🔒 Proposed fix (serialize join)
join = async ({ maxJoinRetries = 3, joinResponseTimeout, rpcRequestTimeout, ...data }: JoinCallData & { maxJoinRetries?: number; joinResponseTimeout?: number; rpcRequestTimeout?: number; } = {}): Promise<void> => { - const callingState = this.state.callingState; - - if ([CallingState.JOINED, CallingState.JOINING].includes(callingState)) { - throw new Error(`Illegal State: call.join() shall be called only once`); - } + return withoutConcurrency(this.joinLeaveConcurrencyTag, async () => { + const callingState = this.state.callingState; + + if ([CallingState.JOINED, CallingState.JOINING].includes(callingState)) { + throw new Error(`Illegal State: call.join() shall be called only once`); + } - if (data.ring) { - this.ringingSubject.next(true); - } + if (data.ring) { + this.ringingSubject.next(true); + } - const callingX = globalThis.streamRNVideoSDK?.callingX; - if (callingX) { - // for Android/iOS, we need to start the call in the callingx library as soon as possible - await callingX.startCall(this); - } - await this.setup(); + const callingX = globalThis.streamRNVideoSDK?.callingX; + if (callingX) { + // for Android/iOS, we need to start the call in the callingx library as soon as possible + await callingX.startCall(this); + } + await this.setup(); - try { + try { ... - } catch (error) { - callingX?.endCall(this); - throw error; - } + } catch (error) { + callingX?.endCall(this); + throw error; + } + }); };As per coding guidelines: Use withoutConcurrency for serial execution in join/leave operations and critical state changes; use withCancellation for device toggling and switching operations.
🤖 Fix all issues with AI agents
In `@packages/client/src/Call.ts`:
- Around line 757-762: The code emits this.ringingSubject.next(true) before
awaiting this.setup(), which causes handleRingingCall() to run before settings
are populated and may skip scheduleAutoDrop(); to fix, move or defer the ringing
emission so it occurs only after setup() and after the call response is applied
(or ensure handleRingingCall() is invoked again once settings arrive): update
the places that call this.ringingSubject.next(true) (refer to the ringingSubject
emission near the top of the call handling flow and the similar emission around
lines 787-792) to emit after await this.setup() / after applyCallResponse()
completes, or add a re-run trigger to handleRingingCall() once settings are
loaded so scheduleAutoDrop() gets scheduled.
- Around line 903-958: The try/catch that ensures callingX?.endCall(this) runs
is too late—move the early startup calls into that protected block so failures
in callingX.startCall(this) or this.setup() will trigger cleanup; specifically,
include the await callingX.startCall(this) and await this.setup() lines inside
the same try that wraps join logic (the block that sets
joinResponseTimeout/rpcRequestTimeout, sfuJoinFailures, for-loop and calls
this.doJoin), so any thrown error will fall through to the existing catch which
calls callingX?.endCall(this) and rethrows.
In `@packages/client/src/types.ts`:
- Line 21: The file currently imports Call as a runtime import which can create
circular dependency issues; change the import to a type-only import (use "import
type { Call } from './Call'") so Call is only emitted as a TypeScript type and
not a runtime require, and verify usages in the StreamRNVideoSDKCallingX type
are purely type positions so no runtime dependency remains.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt`:
- Around line 365-382: The debug log in startBackgroundTaskAutomatically is
inverted: it logs "[module] startBackgroundTaskAutomatically: Headless task
registered, starting automatically" when isHeadlessTaskRegistered is false and
the function returns; update the log to correctly reflect the condition by
changing the message to indicate the headless task is NOT registered (e.g.,
"Headless task not registered, not starting automatically") or flip the if
condition to check isHeadlessTaskRegistered and keep the original message;
modify the debugLog call (referencing TAG, debugLog, and
isHeadlessTaskRegistered in startBackgroundTaskAutomatically) accordingly so the
log matches the actual branch behavior.
- Around line 197-205: The answerIncomingCall method currently hardcodes
isAudioCall=true; instead look up the actual call type from the stored call
attributes (e.g., via CallRepository / registeredCall.callAttributes or the same
place displayIncomingCall sets hasVideo/CALL_TYPE_VIDEO_CALL) and set
isAudioCall = (callType != CALL_TYPE_VIDEO_CALL) before invoking
executeServiceAction(callId, CallAction.Answer(isAudioCall), promise); update
answerIncomingCall to read the call's attributes (or accept a callType
parameter) and derive isAudioCall accordingly so video calls are answered as
video.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.kt`:
- Around line 124-135: The ACTION_START_BACKGROUND_TASK branch in
CallService.onStartCommand currently skips calling startForeground() when
isInForeground is false, risking an FGS timeout crash because
CallingxModule.startBackgroundTask() invokes startForegroundService(); fix by
enforcing the FGS contract: either guard the action to only run when a call is
active or unconditionally start foreground with a minimal notification before
invoking startBackgroundTask(intent). Concretely, update
CallService.onStartCommand's ACTION_START_BACKGROUND_TASK handling so that if
!isInForeground you build a minimal notification (e.g., via
CallNotificationManager or a new helper), call
startForeground(CallNotificationManager.NOTIFICATION_ID, minimalNotification),
set isInForeground = true, then call startBackgroundTask(intent); alternatively,
check for an active call/registered call state and return/ignore the action if
none exists to prevent starting the service as a foreground service without a
call.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt`:
- Around line 25-35: setListener currently starts a new scope.launch collecting
currentCall every call and never cancels prior collectors, causing duplicate
events and coroutine leaks; mirror TelecomCallRepository by adding a nullable
Job property (e.g., callStateCollectorJob) to LegacyCallRepository, cancel and
join/cleanup the previous job if non-null before assigning this._listener, then
start a new scope.launch that assigns to callStateCollectorJob and collects
currentCall calling _listener?.onCallStateChanged(it) inside the
try/catch—ensure you cancel the job when listener is removed or repository is
disposed.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt`:
- Around line 334-359: doAnswer currently marks the call as Unregistered when
answer(callType) returns CallControlResult.Error but never notifies listeners,
so JS misses end-call events; after setting isSelfAnswered = false and calling
updateCurrentCall (Call.Unregistered), also invoke onIsCallDisconnected with an
appropriate DisconnectCause (e.g., DisconnectCause.BUSY or result-specific) to
mirror doDisconnect's behavior; update doAnswer to call
onIsCallDisconnected(cause) immediately after updateCurrentCall to ensure
listeners are notified when answer() fails.
In `@packages/react-native-sdk/src/utils/internal/audioSessionPromise.ts`:
- Around line 16-24: The current waitForAudioSessionActivation() overwrites
pendingResolve/pendingTimeout when called again, orphaning the first promise;
change it to return an existing pending promise instead of creating a new one:
add a module-scoped pendingPromise variable, have
waitForAudioSessionActivation() check and return pendingPromise if set, and only
create/set pendingResolve, pendingTimeout, and pendingPromise when none exists;
ensure resolvePendingAudioSession() clears pendingPromise (and
pendingResolve/pendingTimeout) when resolving or on AUDIO_SESSION_TIMEOUT_MS so
subsequent calls can create a new promise.
In `@packages/react-native-sdk/src/utils/internal/callingx.ts`:
- Around line 60-93: Wrap the native async calls in startCallingxCall and
endCallingxCall with try-catch blocks to prevent unhandled promise rejections:
surround the CallingxModule.startCall call (and the subsequent await
waitForAudioSessionActivation() on iOS) inside a try/catch in startCallingxCall
and log the error via the project's logging mechanism; likewise wrap
CallingxModule.endCallWithReason in endCallingxCall with a try/catch and log any
error. Ensure you still await the calls and do not suppress errors silently—log
the exception with a clear message mentioning CallingxModule.startCall or
CallingxModule.endCallWithReason and the call.cid so debugging is possible.
🧹 Nitpick comments (3)
packages/react-native-sdk/src/modules/call-manager/native-module.d.ts (1)
72-72: Consider adding parameter name for clarity.The parameter lacks a name, which reduces readability and IDE support. This is pre-existing code but could be improved while in this file.
✨ Suggested improvement
- setForceSpeakerphoneOn: (boolean) => void; + setForceSpeakerphoneOn: (force: boolean) => void;packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/audio/utils/WebRtcAudioUtils.kt (1)
65-129: Remove the dead logging methods:logAudioStateBasic(),logAudioStateVolume(), andlogAudioDeviceInfo().These three private methods (lines 213–284) are never called anywhere in the codebase and their logic is now incorporated into
getAudioStateLog(). Removing them eliminates dead code and reduces maintenance burden.packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt (1)
74-85: Optional: silence empty-block warnings.
onHostResume()/onHostPause()are empty; consider= Unitor a suppression to keep detekt clean.
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallService.kt
Show resolved
Hide resolved
...-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt
Show resolved
Hide resolved
...native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/TelecomCallRepository.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/Call.ts (1)
670-676:⚠️ Potential issue | 🟠 MajorPreserve
isRingingTypeCallbefore clearing ringing state.Line 670 clears
ringingSubject, so Line 674 always passesfalsetocallManager.stop(). This can misclassify ringing-type calls and leave native call UI/notifications dangling. Capture the value before clearing it.🛠 Proposed fix
- this.ringingSubject.next(false); + const wasRingingTypeCall = this.ringing; + this.ringingSubject.next(false); ... - globalThis.streamRNVideoSDK?.callManager.stop({ - isRingingTypeCall: this.ringing, - }); + globalThis.streamRNVideoSDK?.callManager.stop({ + isRingingTypeCall: wasRingingTypeCall, + });
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt`:
- Around line 236-259: The updateDisplay method currently calls startCallService
with hasVideo hardcoded to true which can misrepresent the call state; change
updateDisplay to derive the video flag from displayOptions (or add an explicit
parameter) and pass that boolean into startCallService instead of true, or if
video is irrelevant, update the method signature and call to make it clear
(e.g., remove/ignore hasVideo in startCallService) so
CallService.ACTION_UPDATE_CALL receives the correct video state; locate
updateDisplay and the startCallService invocation and adjust the hasVideo
argument accordingly.
- Around line 153-161: Fix the typos in the inline comment inside
getInitialEvents: change "writabel" to "writable" and "immidiate" to "immediate"
so the comment correctly reads that a writable native array can be consumed only
once and that events are cleared and eaten immediately after retrieval; no code
behavior changes required (references: getInitialEvents, delayedEvents,
WritableNativeArray, canSendEvents).
- Around line 411-435: In sendJSEvent, the constructed paramsMap (with proper
Boolean/String coercion) is never emitted because emit(eventName, params) sends
the original params; update the emit call in sendJSEvent to pass the coerced
paramsMap (or null/empty map appropriately) so the type-handling logic in the
Arguments.createMap().apply block is actually delivered to JS; locate
sendJSEvent in CallingxModule.kt and replace the
DeviceEventManagerModule.RCTDeviceEventEmitter.emit call to use paramsMap
instead of params.
- Around line 515-533: The debug/log messages inside the tryToBindIfNeeded
method incorrectly reference checkForExistingService; update the strings passed
to debugLog(...) and Log.e(...) in tryToBindIfNeeded so they mention
tryToBindIfNeeded (or a consistent module tag reflecting this method) instead of
checkForExistingService; locate the calls to debugLog(TAG, "[module]
checkForExistingService: ...") and replace the message text to "[module]
tryToBindIfNeeded: ..." (and adjust the Log.e message similarly) to ensure
accurate method names in logs.
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt`:
- Around line 41-95: registerCall() creates a local Channel called actionSource
and launches a collector job but never tracks or closes them; add instance
properties (e.g., actionChannel: Channel<CallAction>? and actionProcessingJob:
Job?) to LegacyCallRepository, and before creating a new Channel in
registerCall() ensure any existing actionProcessingJob is cancelled and any
existing actionChannel is closed and nulled; assign the new channel to
actionChannel and the launched coroutine to actionProcessingJob, and also
close/cancel and null these same members in release() and wherever a call is
disconnected, ensuring the collector uses actionChannel.consumeAsFlow() ->
processActionLegacy(...) and that exceptions are still logged.
In `@packages/react-native-sdk/src/utils/internal/callingx.ts`:
- Around line 45-50: The fallback that sets names when names.length === 0 should
look for the current user's name in callMembers before checking participants:
search callMembers for an entry whose userId === currentUserId and use its name
if present, otherwise fall back to participants.find(... )?.name and finally
default to 'Call'; update the logic around the names assignment (the names
variable and the currentUserId, callMembers, participants references) so the
fallback matches the comment’s intent and covers early ringing cases where
participants is empty.
🧹 Nitpick comments (3)
packages/react-native-sdk/src/utils/internal/callingx.ts (1)
20-24: Add explicit return types for exported helpers.Make the exported helpers’ return types explicit to keep the public surface self-documenting and consistent.
As per coding guidelines: Use explicit TypeScript types and interfaces for all public API surfaces.♻️ Suggested change
export function getCallDisplayName( callMembers: MemberResponse[] | undefined, participants: StreamVideoParticipant[] | undefined, currentUserId: string | undefined, -) { +): string { if (!callMembers || !participants || !currentUserId) { return 'Call'; } ... } -export async function startCallingxCall(call: Call) { +export async function startCallingxCall(call: Call): Promise<void> { if (!CallingxModule || CallingxModule.isCallRegistered(call.cid)) { return; } ... } -export async function endCallingxCall(call: Call) { +export async function endCallingxCall(call: Call): Promise<void> { if (!CallingxModule || !CallingxModule.isCallRegistered(call.cid)) { return; } ... }Also applies to: 61-61, 96-96
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt (1)
384-409: Minor: Redundant null check.At lines 388-389,
callServiceis checked for null but then accessed with safe call operator?.which is redundant. Consider using direct access since the null check is already done.♻️ Suggested simplification
BindingState.BOUND -> { - if (callService != null) { - callService?.processAction(callId, action) + val service = callService + if (service != null) { + service.processAction(callId, action) promise.resolve(true) } else { promise.reject("ERROR", "Service reference lost") } }packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt (1)
28-39: Avoid collecting call state when no listener is attached.
IfsetListener(null)is called (e.g., teardown), this still launches a coroutine that collects forever. Consider returning early after canceling to avoid unnecessary work.♻️ Suggested tweak
override fun setListener(listener: Listener?) { this._listener = listener // Observe call state changes observeCallStateJob?.cancel() + if (listener == null) { + observeCallStateJob = null + return + } observeCallStateJob = scope.launch { try { currentCall.collect { _listener?.onCallStateChanged(it) } } catch (e: Exception) { Log.e(TAG, "[repository] setListener: Error collecting call state", e) } } }
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt
Show resolved
Hide resolved
| override fun release() { | ||
| _currentCall.value = Call.None | ||
|
|
||
| observeCallStateJob?.cancel() | ||
| observeCallStateJob = null | ||
| _listener = null | ||
|
|
||
| scope.cancel() | ||
| } | ||
|
|
||
| override suspend fun registerCall( | ||
| callId: String, | ||
| displayName: String, | ||
| address: Uri, | ||
| isIncoming: Boolean, | ||
| isVideo: Boolean, | ||
| displayOptions: Bundle?, | ||
| ) { | ||
| registrationMutex.withLock { | ||
| if (currentCall.value is Call.Registered) { | ||
| Log.w( | ||
| TAG, | ||
| "[repository] registerCall: Call already registered, ignoring new call request" | ||
| ) | ||
| return@withLock | ||
| } | ||
|
|
||
| val attributes = createCallAttributes(displayName, address, isIncoming, isVideo) | ||
| val actionSource = Channel<CallAction>() | ||
|
|
||
| _currentCall.value = | ||
| Call.Registered( | ||
| id = callId, | ||
| isActive = false, | ||
| isOnHold = false, | ||
| callAttributes = attributes, | ||
| displayOptions = displayOptions, | ||
| isMuted = false, | ||
| errorCode = null, | ||
| currentCallEndpoint = null, | ||
| availableCallEndpoints = emptyList(), | ||
| actionSource = actionSource, | ||
| ) | ||
|
|
||
| _listener?.onCallRegistered(callId, isIncoming) | ||
|
|
||
| // Process actions without telecom SDK | ||
| scope.launch { | ||
| try { | ||
| actionSource.consumeAsFlow().collect { action -> processActionLegacy(action) } | ||
| } catch (e: Exception) { | ||
| Log.e(TAG, "[repository] registerCall: Error consuming actions", e) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the full LegacyCallRepository.kt file
find . -name "LegacyCallRepository.kt" -type fRepository: GetStream/stream-video-js
Length of output: 177
🏁 Script executed:
# Let's also check the directory structure
ls -la packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/ 2>/dev/null || echo "Directory not found"Repository: GetStream/stream-video-js
Length of output: 399
🏁 Script executed:
# Read the full LegacyCallRepository.kt file
cat -n ./packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.ktRepository: GetStream/stream-video-js
Length of output: 5805
🏁 Script executed:
# Also check the parent class to understand the scope and lifecycle
cat -n ./packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/CallRepository.ktRepository: GetStream/stream-video-js
Length of output: 6328
Explicitly track and close the action channel and processing job.
registerCall() creates a new Channel and collector job but neither is closed/canceled when a new call is registered, when the call disconnects, or when release() is invoked. While scope.cancel() in release() eventually cancels the job, explicit tracking ensures channels are closed immediately, preventing suspended senders and resource leaks. If registerCall() is called multiple times before release(), orphaned channels and jobs accumulate in the scope. Track actionProcessingJob and actionChannel as instance variables and clear them on disconnect, before creating a new registration, and in release().
🔧 Suggested fix
class LegacyCallRepository(context: Context) : CallRepository(context) {
companion object {
private const val TAG = "[Callingx] LegacyCallRepository"
}
private var observeCallStateJob: Job? = null
+ private var actionProcessingJob: Job? = null
+ private var actionChannel: Channel<CallAction>? = null
override fun getTag(): String = TAG
override fun release() {
_currentCall.value = Call.None
observeCallStateJob?.cancel()
observeCallStateJob = null
+ actionProcessingJob?.cancel()
+ actionProcessingJob = null
+ actionChannel?.close()
+ actionChannel = null
_listener = null
scope.cancel()
}
override suspend fun registerCall(
callId: String,
displayName: String,
address: Uri,
isIncoming: Boolean,
isVideo: Boolean,
displayOptions: Bundle?,
) {
registrationMutex.withLock {
if (currentCall.value is Call.Registered) {
Log.w(
TAG,
"[repository] registerCall: Call already registered, ignoring new call request"
)
return@withLock
}
+ actionProcessingJob?.cancel()
+ actionProcessingJob = null
+ actionChannel?.close()
+ actionChannel = null
+
val attributes = createCallAttributes(displayName, address, isIncoming, isVideo)
- val actionSource = Channel<CallAction>()
+ val actionSource = Channel<CallAction>()
+ actionChannel = actionSource
_currentCall.value =
Call.Registered(
id = callId,
isActive = false,
isOnHold = false,
callAttributes = attributes,
displayOptions = displayOptions,
isMuted = false,
errorCode = null,
currentCallEndpoint = null,
availableCallEndpoints = emptyList(),
actionSource = actionSource,
)
_listener?.onCallRegistered(callId, isIncoming)
// Process actions without telecom SDK
- scope.launch {
+ actionProcessingJob = scope.launch {
try {
actionSource.consumeAsFlow().collect { action -> processActionLegacy(action) }
} catch (e: Exception) {
Log.e(TAG, "[repository] registerCall: Error consuming actions", e)
}
}
}
}
private fun processActionLegacy(action: CallAction) {
when (action) {
is CallAction.Answer -> {
updateCurrentCall { copy(isActive = true, isOnHold = false) }
// In legacy mode, all actions are initiated from the app
(currentCall.value as? Call.Registered)?.let {
_listener?.onIsCallAnswered(it.id, EventSource.APP)
}
}
is CallAction.Disconnect -> {
val call = currentCall.value as? Call.Registered
if (call != null) {
_currentCall.value =
Call.Unregistered(call.id, call.callAttributes, action.cause)
// In legacy mode, all actions are initiated from the app
_listener?.onIsCallDisconnected(
call.id,
action.cause,
EventSource.APP
)
+ actionChannel?.close()
+ actionChannel = null
+ actionProcessingJob?.cancel()
+ actionProcessingJob = null
}
}
is CallAction.ToggleMute -> {
updateCurrentCall { copy(isMuted = action.isMute) }
}
// Handle other actions...
else -> {
/* No-op for unsupported actions */
}
}
}Also applies to: 108-137
🤖 Prompt for AI Agents
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/repo/LegacyCallRepository.kt`
around lines 41 - 95, registerCall() creates a local Channel called actionSource
and launches a collector job but never tracks or closes them; add instance
properties (e.g., actionChannel: Channel<CallAction>? and actionProcessingJob:
Job?) to LegacyCallRepository, and before creating a new Channel in
registerCall() ensure any existing actionProcessingJob is cancelled and any
existing actionChannel is closed and nulled; assign the new channel to
actionChannel and the launched coroutine to actionProcessingJob, and also
close/cancel and null these same members in release() and wherever a call is
disconnected, ensuring the collector uses actionChannel.consumeAsFlow() ->
processActionLegacy(...) and that exceptions are still logged.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/src/Call.ts (1)
670-676:⚠️ Potential issue | 🔴 CriticalCapture ringing state before resetting it for
callManager.stop().On line 670,
this.ringingSubject.next(false)resets the ringing state before line 675 reads it for the stop call. Sincethis.ringingusesgetCurrentValue()on the subject, it will always befalseat the point of the stop call. This breaks iOS CallKit detection inshouldBypassForCallKit()— ringing calls won't be recognized as such, causing the native audio manager to stop even when CallKit is handling the call, leading to audio session conflicts.Capture the ringing state before resetting it:
Suggested fix
+ const wasRingingTypeCall = this.ringing; this.ringingSubject.next(false); this.cancelAutoDrop(); this.clientStore.unregisterCall(this); globalThis.streamRNVideoSDK?.callManager.stop({ - isRingingTypeCall: this.ringing, + isRingingTypeCall: wasRingingTypeCall, });
🤖 Fix all issues with AI agents
In
`@packages/react-native-sdk/src/hooks/push/useCallingExpWithCallingStateEffect.ts`:
- Around line 11-29: The helper function canActivateCall is dead code—either
remove it or use it in the calling-state transition effect: if you intend to
detect transitions into JOINING/JOINED, import/keep CallingState and call
canActivateCall(prevState, currentState) inside the effect that handles calling
state changes (e.g., the effect that currently reads calling state or dispatches
call activation) and trigger your call activation logic (start/activate/register
outgoing call) when it returns true; otherwise delete the canActivateCall
declaration to eliminate the unused symbol.
🧹 Nitpick comments (1)
packages/react-native-sdk/src/hooks/push/useCallingExpWithCallingStateEffect.ts (1)
170-237: Commented-out code contains a guideline violation when uncommented.The commented mute synchronization code at lines 208-209 uses
RxUtils.getCurrentValue(microphone.state.status$)which directly accesses RxJS observables. When this code is uncommented, refactor to use the bindings hooks pattern instead.- const isCurrentlyMuted = - RxUtils.getCurrentValue(microphone.state.status$) === 'disabled'; + // Use isMute from useMicrophoneState() hook instead of direct observable access + const isCurrentlyMuted = isMute;Note: This would require restructuring since
isMutefrom the hook reflects the current render cycle, not the real-time value at event time. Consider using a ref to track current mute state, or restructuring the event handler.
packages/react-native-sdk/src/hooks/push/useCallingExpWithCallingStateEffect.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/react-native-callingx/ios/Callingx.mm`:
- Around line 276-278: The canPostNotifications method currently always returns
`@YES`; replace it with an actual check using
UNUserNotificationCenter.currentNotificationCenter
getNotificationSettingsWithCompletionHandler to inspect
settings.authorizationStatus and return `@YES` only for
authorized/provisional/ephemeral (treat as allowed) and `@NO` for
denied/notDetermined; update the method signature/flow so the async result is
delivered (e.g., convert canPostNotifications to an async callback or promise
resolver) and ensure the completion handler marshals back to the calling thread
when returning the NSNumber result for canPostNotifications.
🧹 Nitpick comments (3)
packages/react-native-callingx/ios/Callingx.mm (3)
157-169: UnuseddisplayOptionsparameter.The
displayOptionsparameter is accepted but not passed to the underlying implementation. If this is intentional for iOS/Android API parity, consider adding a comment. Otherwise, pass the options todisplayIncomingCallWithCallId:.
171-177: Inconsistent result handling compared toendCall:.
endCallWithReason:always resolves with@YESwithout checking the result, while the sibling methodendCall:(lines 179-184) properly returns the actual result from_moduleImpl. Consider returning the actual result for consistency.Proposed fix
- (void)endCallWithReason:(nonnull NSString *)callId reason:(double)reason resolve:(nonnull RCTPromiseResolveBlock)resolve reject:(nonnull RCTPromiseRejectBlock)reject { - [CallingxImpl endCall:callId reason:(int)reason]; - resolve(`@YES`); + BOOL result = [_moduleImpl endCallWithReason:callId reason:(int)reason]; + resolve(@(result)); }
217-229: UnuseddisplayOptionsand unconditional success.Similar to
displayIncomingCall, thedisplayOptionsparameter is unused. Additionally, the method always resolves with@YESwithout checking the result fromstartCallWithCallId:. Consider returning the actual result for consistency with other methods.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/react-native-callingx/ios/CallingxImpl.swift`:
- Line 414: The call to CallingxImpl.endCall currently passes
CXCallEndedReason.failed.rawValue which equals 1 and is being mapped to
.remoteEnded in the static endCall(reasonCode:) mapping; change the argument to
0 so the mapping resolves to .failed (i.e. call CallingxImpl.endCall(callId,
reason: 0)) or update the mapping inside CallingxImpl.endCall to correctly
handle CXCallEndedReason.failed.rawValue—reference the static
endCall(reasonCode:) method and the CallingxImpl.endCall invocation to locate
and fix the mismatch.
In `@packages/react-native-sdk/src/utils/internal/callingx.ts`:
- Around line 97-114: The incoming-call branch (isIncomingCall) calls native
async functions without error handling; wrap the calls to
CallingxModule.displayIncomingCall(call.cid,...), await
waitForDisplayIncomingCall(call.cid),
CallingxModule.answerIncomingCall(call.cid), and await
waitForAudioSessionActivation() in a try-catch around the entire incoming-call
flow (the isIncomingCall block) so any rejections are caught; in the catch, log
the error (including the error object) and perform appropriate fallback/cleanup
(e.g., don't leave call marked as displayed or attempt further actions), and
ensure control returns/throws a handled error to avoid unhandled promise
rejections.
🧹 Nitpick comments (1)
packages/react-native-sdk/src/utils/internal/callingx.ts (1)
66-66: Minor typo:isOutcomingCallshould likely beisOutgoingCall.The standard term is "outgoing" rather than "outcoming" for calls initiated by the current user.
✏️ Suggested fix
- const isOutcomingCall = call.ringing && call.isCreatedByMe; + const isOutgoingCall = call.ringing && call.isCreatedByMe;Update references at lines 75 accordingly.
| } else if (isIncomingCall) { | ||
| if (!CallingxModule.isCallRegistered(call.cid)) { | ||
| await CallingxModule.displayIncomingCall( | ||
| call.cid, // unique id for call | ||
| call.id, // phone number for display in dialer (we use call id as phone number) | ||
| callDisplayName, // display name for display in call screen | ||
| call.state.settings?.video?.enabled ?? false, // is video call? | ||
| ); | ||
|
|
||
| await waitForDisplayIncomingCall(call.cid); | ||
| } else { | ||
| await CallingxModule.answerIncomingCall(call.cid); | ||
| } | ||
|
|
||
| if (Platform.OS === 'ios') { | ||
| await waitForAudioSessionActivation(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Wrap incoming call path native calls in try-catch to prevent unhandled rejections.
The incoming call path (lines 97-114) has multiple async native module calls without try-catch protection:
displayIncomingCall(line 99)waitForDisplayIncomingCallwhich can reject on timeout (line 106)answerIncomingCall(line 108)waitForAudioSessionActivation(line 112)
Unlike the outgoing call path (lines 78-96) which properly wraps calls, these can cause unhandled promise rejections that may crash the app or leave call state inconsistent. As per coding guidelines, native module calls should always be wrapped in try-catch blocks.
🔧 Suggested fix
} else if (isIncomingCall) {
+ try {
if (!CallingxModule.isCallRegistered(call.cid)) {
await CallingxModule.displayIncomingCall(
call.cid, // unique id for call
call.id, // phone number for display in dialer (we use call id as phone number)
callDisplayName, // display name for display in call screen
call.state.settings?.video?.enabled ?? false, // is video call?
);
await waitForDisplayIncomingCall(call.cid);
} else {
await CallingxModule.answerIncomingCall(call.cid);
}
if (Platform.OS === 'ios') {
await waitForAudioSessionActivation();
}
+ } catch (error) {
+ videoLoggerSystem
+ .getLogger('startCallingxCall')
+ .error(`Error handling incoming call in callingx: ${call.cid}`, error);
+ }
}🤖 Prompt for AI Agents
In `@packages/react-native-sdk/src/utils/internal/callingx.ts` around lines 97 -
114, The incoming-call branch (isIncomingCall) calls native async functions
without error handling; wrap the calls to
CallingxModule.displayIncomingCall(call.cid,...), await
waitForDisplayIncomingCall(call.cid),
CallingxModule.answerIncomingCall(call.cid), and await
waitForAudioSessionActivation() in a try-catch around the entire incoming-call
flow (the isIncomingCall block) so any rejections are caught; in the catch, log
the error (including the error object) and perform appropriate fallback/cleanup
(e.g., don't leave call marked as displayed or attempt further actions), and
ensure control returns/throws a handled error to avoid unhandled promise
rejections.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/noise-cancellation-react-native/package.json`:
- Line 51: Update the release notes/changelog to document the breaking
peerDependency bump for `@stream-io/react-native-webrtc`: explicitly state that
the minimum required version changed from >=125.3.0 to >=137.1.1, list that
consumers on versions 125.3.0 through 137.1.0 will need to upgrade, and include
a short migration note (recommended upgrade command and potential compatibility
considerations); reference the change found in package.json under the
peerDependencies entry for "@stream-io/react-native-webrtc" so reviewers can
verify the documented version range matches the diff.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt`:
- Around line 345-363: The calls to ContextCompat.startForegroundService are
unguarded and can throw ForegroundServiceStartNotAllowedException or
SecurityException; wrap each invocation (in startCallService and the other
places that call ContextCompat.startForegroundService — displayIncomingCall,
startCall, updateDisplay, startBackgroundTask, stopBackgroundTask,
startBackgroundTaskAutomatically) in a try-catch, catching
ForegroundServiceStartNotAllowedException, SecurityException and a general
Exception, and handle failures by rejecting or resolving the corresponding
Promise with a clear error (for methods with promises like
displayIncomingCall/startCall/updateDisplay/startBackgroundTask/stopBackgroundTask)
and by logging/returning an error status for startBackgroundTaskAutomatically
which has no promise; ensure you reference and call the original Intent-building
logic (startCallService) unchanged but perform the
ContextCompat.startForegroundService call inside the try block so exceptions are
propagated to the promise reject path.
In `@packages/react-native-callingx/src/CallingxModule.ts`:
- Around line 253-256: The implementation of stopBackgroundTask in
CallingxModule (method stopBackgroundTask()) doesn't match ICallingxModule's
signature which currently requires stopBackgroundTask(taskName: string):
Promise<void>; update the interface declaration in types.ts to remove the unused
taskName parameter so it reads stopBackgroundTask(): Promise<void>, ensuring the
class correctly implements ICallingxModule and types align with the hard-coded
HEADLESS_TASK_NAME usage in NativeCallingModule.stopBackgroundTask.
In `@packages/react-native-callingx/src/types.ts`:
- Line 122: The interface method stopBackgroundTask currently declares a
taskName parameter but the implementation in CallingxModule (which uses
HEADLESS_TASK_NAME and accepts no args) ignores it; update the types.ts
declaration to remove the parameter so the signature becomes
stopBackgroundTask(): Promise<void> to match CallingxModule.ts, and search for
any other type references to stopBackgroundTask to ensure callers/signatures are
consistent with the no-argument form.
- Around line 174-180: The Omit on iOSOptions incorrectly includes a
non-existent key 'setupAudioSession' from InternalIOSOptions; remove
'setupAudioSession' from the Omit list so iOSOptions only omits
'maximumCallsPerCallGroup', 'maximumCallGroups', and 'handleType'. Update the
type alias iOSOptions accordingly (referencing the iOSOptions and
InternalIOSOptions symbols and the string literal 'setupAudioSession') to
eliminate the stale, confusing entry.
In `@packages/react-native-sdk/src/utils/push/android.ts`:
- Around line 387-389: The notification title and body are recomputed in the
await notifee.displayNotification call by re-calling getTitle and getBody
instead of using the already-computed title and body variables; update the call
to notifee.displayNotification (the invocation that currently calls
getTitle/getBody) to pass the existing title and body variables so the
precomputed values are reused and you avoid duplicate calls or potential side
effects from getTitle/getBody.
🧹 Nitpick comments (12)
packages/react-native-callingx/android/src/main/AndroidManifest.xml (1)
21-26: Consider adding a code comment explainingstopWithTask="true"behavior.The
stopWithTask="true"setting is intentional: CallService relies on the Telecom SDK's CallStyle notification to manage call lifecycle through system-level foreground service delegation, rather than keeping the service alive directly. When the app task is removed, the Telecom framework continues managing the active call independently.However, adding a brief comment in the manifest explaining this rationale would help future maintainers understand why this configuration doesn't cause call drops.
packages/react-native-callingx/src/CallingxModule.ts (3)
62-109:setup()does not guard native calls with try-catch.All
NativeCallingModulecalls throughout this class (e.g.,setupiOS,setupAndroid,setShouldRejectCallWhenBusy) are unguarded. If the native side rejects, the error propagates uncaught. This applies to the entire file, butsetup()is a good entry point to address it since a failure here leaves the module in an inconsistent state —_isSetupis never set totrue, yetsetShouldRejectCallWhenBusyand possiblyregisterHeadlessTaskmay have already executed.Consider at minimum wrapping the body of
setup()so that partial side-effects don't leave the module half-initialized.Sketch
setup(options: CallingExpOptions): void { if (this._isSetup) { return; } + try { this._isOngoingCallsEnabled = options.enableOngoingCalls ?? false; this.setShouldRejectCallWhenBusy(options.shouldRejectCallWhenBusy ?? false); // ... platform-specific setup ... this._isSetup = true; + } catch (error) { + // Reset any partial state + this._isOngoingCallsEnabled = false; + throw error; + } }Based on learnings: "Implement React Native native modules using
NativeModulesAPI and always wrap calls in try-catch blocks to handle promise rejection from native code."
262-285:addEventListenerrelies onas anycasts, erasing type safety at the boundary.The type-level conditional on the callback parameter is well-structured, but the runtime implementation casts to
anytwice (Lines 278, 282), bypassing the type narrowing. This is a pragmatic trade-off given the two separateEventManagerinstances, but it means a caller could register a handler with mismatched params and the compiler wouldn't catch it at the cast site.A safer approach: narrow with an overload pair so each branch gets its own typed call, and reserve
as anyonly for the internal union:Optional overload approach
addEventListener<T extends EventName>( eventName: T, callback: EventListener<EventParams[T]>, ): { remove: () => void }; addEventListener<T extends VoipEventName>( eventName: T, callback: EventListener<VoipEventParams[T]>, ): { remove: () => void }; addEventListener(eventName: EventName | VoipEventName, callback: EventListener<any>): { remove: () => void } { const manager = isVoipEvent(eventName) ? this.voipEventManager : this.eventManager; (manager as EventManager<any, any>).addListener(eventName, callback); return { remove: () => { (manager as EventManager<any, any>).removeListener(eventName, callback); }, }; }
292-293: Singleton export shadows themoduleglobal in Node/RN environments.The local variable is named
module, which shadows the CommonJSmoduleglobal. While this is unlikely to cause a runtime issue in an ESM/bundled context, it can confuse tooling and developers.Rename suggestion
-const module = new CallingxModule(); -export default module; +const callingxModule = new CallingxModule(); +export default callingxModule;packages/react-native-callingx/src/types.ts (1)
230-238:EventDataandVoipEventDatause indexed-access union types that lose per-event correlation.
EventData.paramsis typed asEventParams[EventName], which resolves to the union of all event param types. This means anEventDatavalue witheventName: 'answerCall'could haveparamstyped as the endCall params without a type error. A discriminated union would preserve the correlation:Sketch of a discriminated union approach
export type EventData = { [K in EventName]: { eventName: K; params: EventParams[K] }; }[EventName];Same for
VoipEventData.This would let consumers narrow on
eventNameand get the correctparamstype automatically.packages/react-native-sdk/src/utils/push/android.ts (5)
229-229: Use block-body arrows inforEachto avoid implicit return values.Biome flags these
forEachcallbacks because(fn) => fn()implicitly returns the result offn(). While not a runtime bug, this is easily silenced by using block bodies.♻️ Proposed fix (apply at each occurrence)
- unsubscribeFunctions.forEach((fn) => fn()); + unsubscribeFunctions.forEach((fn) => { fn(); });Also applies to: 240-240, 258-258, 273-273, 304-304
357-357: Avoid magic number1forauthorizationStatus.Use the named constant from the notifee library (e.g.,
notifeeLib.AuthorizationStatus.AUTHORIZED) instead of the hardcoded1. Same applies toimportance: 4on line 396 — prefernotifeeLib.AndroidImportance.HIGH.
317-322: Error is only logged to callingx, not the SDK logger.When the background task fails (line 317), the error is logged via
callingx.logbut not via the SDKlogger. This is inconsistent with the rest of the function and may cause errors to be invisible if callingx logging isn't configured on the native side.♻️ Proposed fix
} catch (error) { + logger.error( + `Failed to start background task with callCid: ${call_cid}`, + error, + ); callingx.log( `Failed to start background task with callCid: ${call_cid} error: ${error}`, 'error', ); finishBackgroundTask();
182-326: Background task Promise never rejects — errors are silently swallowed.The IIFE at line 193 catches all errors and calls
finishBackgroundTask()(which resolves the promise), so the Promise returned toregisterBackgroundTaskcan never reject. IfregisterBackgroundTaskrelies on rejection to signal failure, this could mask issues. If this is intentional (the native side only cares about completion), it's acceptable but worth documenting with a comment.
165-165: ExposerejectCallWhenBusythrough a public getter onStreamVideoClient.The code accesses a private property via bracket notation (
client['rejectCallWhenBusy']), which bypasses TypeScript's type safety. Add a public getter method toStreamVideoClientto provide safe, type-checked access to this property instead.packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt (2)
395-401: Debug log says "queueing action" but the promise is actually rejected.Line 396 logs "Service binding, queueing action" yet the very next statement rejects the promise. No queuing happens.
📝 Suggested fix
BindingState.BINDING -> { - debugLog(TAG, "executeServiceAction: Service binding, queueing action") + debugLog(TAG, "[module] executeServiceAction: Service still binding, rejecting action") promise.reject( "SERVICE_BINDING", "Service is connecting, please try again in a moment" ) }
498-506: SwallowedIllegalArgumentException— include the exception in the log.The
catchat line 501 logs a static message but drops the exception object, hiding the root cause in production diagnostics.📝 Suggested fix
} catch (e: IllegalArgumentException) { Log.w( TAG, - "[module] unbindServiceSafely: Service not registered or already unbound" + "[module] unbindServiceSafely: Service not registered or already unbound", + e ) }
| private fun startCallService( | ||
| action: String, | ||
| callId: String, | ||
| callerName: String, | ||
| phoneNumber: String, | ||
| hasVideo: Boolean, | ||
| displayOptions: ReadableMap? | ||
| ) { | ||
| Intent(reactApplicationContext, CallService::class.java) | ||
| .apply { | ||
| this.action = action | ||
| putExtra(CallService.EXTRA_CALL_ID, callId) | ||
| putExtra(CallService.EXTRA_NAME, callerName) | ||
| putExtra(CallService.EXTRA_URI, phoneNumber.toUri()) | ||
| putExtra(CallService.EXTRA_IS_VIDEO, hasVideo) | ||
| putExtra(CallService.EXTRA_DISPLAY_OPTIONS, Arguments.toBundle(displayOptions)) | ||
| } | ||
| .also { ContextCompat.startForegroundService(reactApplicationContext, it) } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "CallingxModule.kt" | head -20Repository: GetStream/stream-video-js
Length of output: 166
🏁 Script executed:
git ls-files | grep -i "callingx" | head -20Repository: GetStream/stream-video-js
Length of output: 1469
🌐 Web query:
Android ForegroundServiceStartNotAllowedException when thrown API 31
💡 Result:
android.app.ForegroundServiceStartNotAllowedException was introduced with Android 12 / API 31 and is thrown when your app (targeting 31+) calls startForegroundService() while the app is considered “in the background” and the start doesn’t qualify for an allowed exemption. (developer.android.com)
Common ways you hit it (API 31+)
- Starting an FGS from a BroadcastReceiver that runs while your app isn’t visible (e.g., periodic widget updates, implicit broadcasts, background alarms). (developer.android.com)
- Starting an FGS after an FCM message that isn’t effectively high priority (Android can downgrade messages; then the exemption doesn’t apply). (developer.android.com)
Fixes (what to do instead)
- Don’t start a foreground service from background. Move the start to a moment when the app is user-visible (e.g., from an Activity), or after a direct user action (notification action, widget click, etc.). (developer.android.com)
- For background work, use WorkManager / JobScheduler (this is the typical migration path for “do work later in the background”). (developer.android.com)
- If you rely on WorkManager’s foreground mode, ensure you’re on a recent version (Google notes fixes for some related foreground-service crashes in newer WorkManager versions). (developer.android.com)
How to confirm why it happened
Log and inspect the exception (and any chained cause) to see the context that triggered the disallowed start. (developer.android.com)
If you share where you call startForegroundService() (Activity? receiver? widget update? alarm? FCM?), I can tell you which exemption applies (if any) and the most direct replacement (WorkManager vs user-initiated start).
Citations:
- 1: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
- 2: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
- 3: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
- 4: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
- 5: https://developer.android.com/develop/background-work/services/fgs/troubleshooting?utm_source=openai
- 6: https://developer.android.com/develop/background-work/services/fgs/troubleshooting?utm_source=openai
- 7: https://developer.android.com/reference/android/app/ServiceStartNotAllowedException?utm_source=openai
🏁 Script executed:
cat -n packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt | sed -n '340,390p'Repository: GetStream/stream-video-js
Length of output: 2620
🏁 Script executed:
cat -n packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt | grep -E "(displayIncomingCall|startCall|updateDisplay|startBackgroundTask|stopBackgroundTask)" | head -30Repository: GetStream/stream-video-js
Length of output: 1150
🏁 Script executed:
cat -n packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt | sed -n '200,330p'Repository: GetStream/stream-video-js
Length of output: 6209
🏁 Script executed:
cat -n packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt | sed -n '168,205p'Repository: GetStream/stream-video-js
Length of output: 1745
🌐 Web query:
ContextCompat.startForegroundService exceptions thrown IllegalStateException SecurityException
💡 Result:
ContextCompat.startForegroundService() can fail mainly in two ways:
1) IllegalStateException (often ForegroundServiceStartNotAllowedException)
Meaning: You tried to start a foreground service while your app is in the background (Android 12+/API 31+ restrictions). The system blocks it unless you qualify for a specific exemption (for example, it’s immediately triggered from a user-visible state). (developer.android.com)
Fix options:
- Start the FGS only while the app is foreground/user-visible (e.g., directly from an Activity / user action). (developer.android.com)
- If you truly need background work, prefer WorkManager / JobScheduler patterns instead of an FGS-from-background flow (Android’s FGS docs strongly push you to redesign rather than rely on exemptions). (developer.android.com)
- If it’s an allowed exemption case, ensure you’re actually meeting that exemption (Android 12+ has a narrow set). (developer.android.com)
2) SecurityException
Meaning: You’re missing required permissions / declarations for foreground services (and on Android 14+/API 34+, missing type-specific permissions or prerequisites). (developer.android.com)
Common causes + fixes:
- Missing base permission (Android 9+/API 28+):
- Add to manifest:
android.permission.FOREGROUND_SERVICE. (developer.android.com)
- Add to manifest:
- Android 14+ (targetSdk 34+) type enforcement:
- Declare the service’s
android:foregroundServiceType="..."in the<service>manifest entry. - Add the matching manifest permission(s) like
FOREGROUND_SERVICE_LOCATION,FOREGROUND_SERVICE_CAMERA, etc. (developer.android.com) - Also satisfy runtime prerequisites for that type (e.g., location FGS requires location permission). (developer.android.com)
- Declare the service’s
- While-in-use permission trap (Android 14+):
- If the FGS type needs a while-in-use permission (location/camera/mic/body sensors), starting it while your app is background can throw
SecurityExceptioneven if you previously “have” the permission—because it’s only valid while in use. Start it while an Activity is visible (unless a defined exemption applies). (developer.android.com)
- If the FGS type needs a while-in-use permission (location/camera/mic/body sensors), starting it while your app is background can throw
Quick checklist
- Manifest has
FOREGROUND_SERVICE(API 28+). (stackoverflow.com) - For targetSdk 34+: manifest declares
foregroundServiceTypeand the matchingFOREGROUND_SERVICE_*permission(s). (developer.android.com) - Don’t start FGS from background on API 31+ unless you qualify; otherwise redesign to scheduled work. (developer.android.com)
- If using location/camera/mic/body sensors types on Android 14+: start while app is visible (or you’ll hit
SecurityException). (developer.android.com)
If you paste the exact stack trace lines (exception class + message + Android version/targetSdk + your foregroundServiceType), I can tell you which specific rule you’re hitting.
Citations:
- 1: https://developer.android.com/about/versions/12/foreground-services?utm_source=openai
- 2: https://developer.android.com/about/versions/12/foreground-services?utm_source=openai
- 3: https://developer.android.com/develop/background-work/services/fgs/troubleshooting?utm_source=openai
- 4: https://developer.android.com/about/versions/12/foreground-services?utm_source=openai
- 5: https://developer.android.com/develop/background-work/services/fgs/troubleshooting?utm_source=openai
- 6: https://developer.android.com/develop/background-work/services/fgs/troubleshooting?utm_source=openai
- 7: https://developer.android.com/develop/background-work/services/fgs/declare?utm_source=openai
- 8: https://developer.android.com/develop/background-work/services/fgs/launch?utm_source=openai
- 9: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
- 10: https://stackoverflow.com/questions/52382710/permission-denial-startforeground-requires-android-permission-foreground-servic?utm_source=openai
- 11: https://developer.android.com/about/versions/14/changes/fgs-types-required?utm_source=openai
- 12: https://developer.android.com/about/versions/12/foreground-services?utm_source=openai
- 13: https://developer.android.com/develop/background-work/services/fgs/restrictions-bg-start?utm_source=openai
startForegroundService calls are unguarded and can crash the app.
ContextCompat.startForegroundService() throws ForegroundServiceStartNotAllowedException (Android 12+/API 31+) when called from the background, and SecurityException when permissions are missing or improperly declared. This method is called by displayIncomingCall, startCall, and updateDisplay—all of which resolve the promise after the call. An uncaught exception will crash the app before the promise is settled. The same unguarded pattern appears in startBackgroundTask (line 305), stopBackgroundTask (line 316), and startBackgroundTaskAutomatically (line 381).
Wrap each ContextCompat.startForegroundService() call in try-catch and propagate errors to the respective promises (or handle errors appropriately for startBackgroundTaskAutomatically, which has no promise).
🤖 Prompt for AI Agents
In
`@packages/react-native-callingx/android/src/main/java/io/getstream/rn/callingx/CallingxModule.kt`
around lines 345 - 363, The calls to ContextCompat.startForegroundService are
unguarded and can throw ForegroundServiceStartNotAllowedException or
SecurityException; wrap each invocation (in startCallService and the other
places that call ContextCompat.startForegroundService — displayIncomingCall,
startCall, updateDisplay, startBackgroundTask, stopBackgroundTask,
startBackgroundTaskAutomatically) in a try-catch, catching
ForegroundServiceStartNotAllowedException, SecurityException and a general
Exception, and handle failures by rejecting or resolving the corresponding
Promise with a clear error (for methods with promises like
displayIncomingCall/startCall/updateDisplay/startBackgroundTask/stopBackgroundTask)
and by logging/returning an error status for startBackgroundTaskAutomatically
which has no promise; ensure you reference and call the original Intent-building
logic (startCallService) unchanged but perform the
ContextCompat.startForegroundService call inside the try block so exceptions are
propagated to the promise reject path.
3 participants
💡 Overview
Current pull request provides implementation of CallKit/Android Telecom functionality which includes the following:
notifeeimplementationAs part of the PR following dependencies became redundant:
react-native-voip-push-notificationreact-native-callkeepNow neither ringing flow nor call alive functionality don't depend on
@notifee/react-nativewhich will allow to get rid of that dependency in near future.📝 Implementation notes
🎫 Ticket: https://linear.app/stream/issue/RN-17/android-support-for-telecom-manager
📑 Docs: https://github.com/GetStream/docs-content/pull/881
Summary by CodeRabbit
New Features
Improvements
Dependencies