fix: prevent crash on Android devices without telecom feature#7334
fix: prevent crash on Android devices without telecom feature#7334diegolmello wants to merge 2 commits into
Conversation
RNCallKeep.setup() runs unconditionally on Android boot and calls TelecomManager.registerPhoneAccount, which throws UnsupportedOperationException on devices that ship without the android.software.telecom feature (e.g. Zebra TC21 and other rugged enterprise SKUs). The exception lands on the RN bridge thread, so the .catch() in JS cannot intercept it and the process crashes at boot. Patch react-native-callkeep so registerPhoneAccount checks PackageManager.FEATURE_TELECOM up front and returns early, with a try/catch around the underlying TelecomManager call as a second line of defense for OEMs that lie about the feature.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughDefensive updates to react-native-callkeep: Android phone-account registration and availability checks are wrapped and gated by FEATURE_TELECOM; two React bridge overloads were removed; VoiceConnectionService access to RNCallKeep is null-guarded and Android O+ connections set self-managed; iOS delayed events array is initialized when nil. Changesreact-native-callkeep Robustness and API Updates
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
diegolmello
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_ios_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
Belt-and-suspenders to the native patch: skip the JS-side setup entirely on devices that lack android.software.telecom so the RN bridge thread never reaches the Telecom API. Uses react-native-device-info's hasSystemFeatureSync (a true blocking-synchronous bridge method), so cold-start cost is sub-millisecond — the FCM-driven call accept path is unaffected on telecom-capable devices. The native patch is kept as defense-in-depth for OEMs that misreport the feature and for any future call site that bypasses this gate.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
index.js (1)
26-26: ⚡ Quick winThe
hasSystemFeatureSyncAPI is documented and available in react-native-device-info 11.1.0—no blocking issue.Verification confirms that
DeviceInfo.hasSystemFeatureSync(feature)is a documented synchronous method in react-native-device-info 11.1.0 that returns a boolean directly. The method exists and is safe for use during app initialization.If you want additional robustness, consider wrapping the feature check in try-catch as a secondary safeguard:
- if (Platform.OS === 'android' && DeviceInfo.hasSystemFeatureSync('android.software.telecom')) { + let hasTelecomFeature = false; + try { + hasTelecomFeature = Platform.OS === 'android' && DeviceInfo.hasSystemFeatureSync('android.software.telecom'); + } catch (error) { + console.warn('Failed to check telecom feature availability:', error); + } + + if (hasTelecomFeature) {This is optional but recommended for defense-in-depth on synchronous bridge calls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@index.js` at line 26, The DeviceInfo.hasSystemFeatureSync call can throw in rare cases even though it exists; wrap the Android-only check (where Platform.OS === 'android' and DeviceInfo.hasSystemFeatureSync(...)) in a try/catch, store the result to a local boolean (e.g., hasTelecomFeature) and default to false on error, then use that boolean in the conditional; reference DeviceInfo.hasSystemFeatureSync and the Platform.OS check so the change is made exactly around that expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@index.js`:
- Line 26: The DeviceInfo.hasSystemFeatureSync call can throw in rare cases even
though it exists; wrap the Android-only check (where Platform.OS === 'android'
and DeviceInfo.hasSystemFeatureSync(...)) in a try/catch, store the result to a
local boolean (e.g., hasTelecomFeature) and default to false on error, then use
that boolean in the conditional; reference DeviceInfo.hasSystemFeatureSync and
the Platform.OS check so the change is made exactly around that expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e94bda7b-8356-47e8-a914-0fb6488ee3c0
📒 Files selected for processing (1)
index.js
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
index.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use Prettier with tabs, single quotes, 130 char width, no trailing commas, arrow parens avoid, bracket same line
Use@rocket.chat/eslint-configbase with React, React Native, TypeScript, Jest plugins
Files:
index.js
index.js
📄 CodeRabbit inference engine (CLAUDE.md)
App registration and conditional Storybook loading should be in index.js
Files:
index.js
🔇 Additional comments (1)
index.js (1)
5-5: LGTM!
diegolmello
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_ios_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
upload_experimental_android
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.73.0.108919 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNR9FQXgQEJmPasQWObsdHEn6yAQj-Q50XoPlgUd5yid1soVhTMQd786BQ_BdZRxx_w_ZabIPP5ck-Majq3F |
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
official_ios_build
GitHub Actions
Waiting
diegolmello
requested a deployment
to
upload_experimental_android
GitHub Actions
Waiting
1 participant
Proposed changes
RNCallKeep.setup()runs unconditionally fromindex.json every Android boot and ends up callingTelecomManager.registerPhoneAccount. On devices that ship without theandroid.software.telecomsystem feature — e.g. Zebra TC21 and other rugged enterprise SKUs — that call throwsUnsupportedOperationException. The exception lands on the RN bridge thread (mqt_v_native), so the.catch(...)attached to the JS-sidesetup()promise cannot intercept it and the process crashes immediately at startup.Two layers of defense:
index.js) — skipRNCallKeep.setup()when the device lacksandroid.software.telecom. Usesreact-native-device-info'shasSystemFeatureSync(a true blocking-synchronous bridge method, sub-millisecond), so the cold-start path that accepts FCM-driven calls is unaffected on telecom-capable devices. No new native module needed;react-native-device-infois already a dependency.patches/react-native-callkeep+4.3.16.patch) —RNCallKeepModule.registerPhoneAccountnow checksPackageManager.FEATURE_TELECOMand returns early, with atry/catcharound the underlyingtelecomManager.registerPhoneAccount(...)call. Kept as defense-in-depth for OEMs that misreport the feature and for any future call site that bypasses the JS gate.The existing native
VoipNotification.ktpath already catchesExceptionaround its ownregisterPhoneAccount, so no change is needed there. VoIP is not usable on these devices anyway — this just stops the boot crash so the rest of the app continues to function.Issue(s)
How to test or reproduce
Reproduces deterministically on any Android device whose
adb shell pm list featuresoutput does not includeandroid.software.telecom. Confirmed on the Zebra TC21 (Android 13) in the linked ticket. After the fix, the app should boot cleanly on that device — no Telecom call is attempted from JS, and even if something else were to reachRNCallKeepModule.registerPhoneAccount, the native guard logs[RNCallKeepModule] registerPhoneAccount skipped: device lacks FEATURE_TELECOMinstead of crashing.A standard Android emulator (Pixel images) does include the telecom feature, so emulator builds are not a substitute repro — they only verify the no-regression case on telecom-capable devices.
Screenshots
N/A — boot crash fix, no UI change.
Types of changes
Checklist
Further comments
No JS unit test added: the gate is a single boolean check against a native-platform predicate, with no good seam to test in isolation that would be more meaningful than just reading the code. The native patch re-applies cleanly against an unmodified
react-native-callkeep@4.3.16tarball (verified withpatch -p1 --dry-run).Summary by CodeRabbit