fix(android): handle JNI exceptions during initConnection setup#3158
Conversation
Wrap setActivity and listener registration in try-catch to convert raw JNI exceptions into structured OpenIapException with proper error code and message. Previously, exceptions from OpenIapModule lazy initialization or listener registration propagated unhandled through Promise.async, producing cryptic "Unknown N8facebook3jni12JniExceptionE error" messages on the JS side. Now developers receive structured errors with code "init-connection" and descriptive messages identifying the failure point. Closes #3144 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness of the Android in-app purchase initialization process by proactively catching and handling potential JNI exceptions that could occur during the setup of the IAP module. By converting these low-level errors into a consistent, structured format, it provides a much better developer experience, making it easier to diagnose and resolve issues related to Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGuards Activity retrieval and billing-listener registration in initConnection with try/catch blocks. On failures, logs errors, resets listener state, cancels pending initialization deferreds, and throws structured OpenIapException (JSON) to propagate failures to awaiters and concurrent callers. Changes
Sequence Diagram(s)sequenceDiagram
actor RNModule as ReactNative Module
participant Activity as Android Activity
participant Billing as BillingClient
participant Listeners as Billing Listeners
RNModule->>Activity: retrieve Activity (main thread)
alt Activity obtained
RNModule->>Billing: init connection
RNModule->>Billing: attach listeners
Billing->>Listeners: register listeners
Listeners-->>RNModule: success
else Activity retrieval fails
Activity-->>RNModule: Throwable
RNModule->>RNModule: log error
RNModule->>RNModule: throw OpenIapException (error JSON)
end
alt Listener registration throws
Billing-->>RNModule: error
RNModule->>RNModule: set listenersAttached = false
RNModule->>RNModule: cancel/clear pending init deferreds
RNModule->>RNModule: log error
RNModule->>RNModule: throw OpenIapException (registration failure JSON)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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}
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 `@android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt`:
- Around line 118-128: The catch blocks that currently catch Throwable (in the
initConnection.setActivity handler and the other similar catch in
HybridRnIap.kt) should rethrow Kotlin's CancellationException to preserve
coroutine cancellation semantics before converting exceptions to
OpenIapException; update the catch to check if (err is CancellationException)
throw err, then proceed to log with RnIapLog.failure and throw the
OpenIapException created via toErrorJson (keep the existing error, debugMessage,
and messageOverride logic) so cancellation is not swallowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8c11d6e-676d-48e6-b0b7-c2bea4307cdc
📒 Files selected for processing (1)
android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses unhandled JNI exceptions during the initConnection process by wrapping critical setup calls in try-catch blocks, converting them into structured OpenIapException objects, and improving error reporting on the JavaScript side. However, a security concern was identified regarding the logging utility (RnIapLog), which has incomplete sanitization logic and could potentially leak sensitive information to the system logs. It is recommended to enhance the sanitization to include a broader range of sensitive keywords and to ensure that non-map objects are also properly handled before being logged. Additionally, minor suggestions have been made to reduce code duplication within the newly added catch blocks for better maintainability.
- Add dedicated CancellationException catch blocks before Throwable to preserve coroutine cancellation semantics in both setActivity and listeners try-catch blocks - Extract `err.message ?: err.javaClass.name` into `errorMessage` variable to avoid duplicate expressions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 `@android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt`:
- Around line 147-221: The listener-registration try/catch can throw before
initDeferred (the Deferred created earlier) is completed/cleared, causing
concurrent initConnection callers to deadlock; update the initConnection flow so
that any thrown Throwable in the listeners block completes exceptionally and
clears/reset initDeferred and listenersAttached before rethrowing—best done by
moving listener registration into a try/finally (or ensure the catch completes
initDeferred.completeExceptionally(err) and sets initDeferred = null and
listenersAttached = false) so callers waiting on initDeferred never wait
forever; refer to symbols initDeferred, listenersAttached,
OpenIap.addPurchaseUpdateListener, addPurchaseErrorListener,
addUserChoiceBillingListener, addDeveloperProvidedBillingListener and the
surrounding initConnection logic to implement the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8456ace3-94aa-4603-ba70-9bea6785a12f
📒 Files selected for processing (1)
android/src/main/java/com/margelo/nitro/iap/HybridRnIap.kt
If listener registration throws, complete initDeferred exceptionally and reset it to prevent concurrent initConnection callers from deadlocking on await(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
setActivityand listener registration in try-catch blocks to convert raw JNI exceptions into structuredOpenIapExceptionUnknown N8facebook3jni12JniExceptionE errormessages on the JS sideinit-connectionand descriptive messagesProblem
During
initConnection, theopenIapobject is lazy-initialized (OpenIapModule(context)). The first access happens at eitheropenIap.setActivity()oropenIap.addPurchaseUpdateListener()— both were outside any try-catch block.On devices without Google Play Services or with billing client issues, the constructor throws a JNI exception that propagates unhandled through
Promise.async. Nitro converts the raw C++ exception class name (facebook::jni::JniException) into the unhelpful error message users see.Meanwhile, the actual
openIap.initConnection()call (which IS protected by try-catch) is never reached because the exception occurs earlier.Changes
Android (
android/.../HybridRnIap.kt)setActivitycall in try-catch → throwsOpenIapExceptionwith message"Failed to set activity: <original error>"OpenIapExceptionwith message"Failed to register billing listeners: <original error>"listenersAttached = falseon listener registration failure for retry on nextinitConnectioncallinit-connectionerror code for consistent JS-side handlingTest plan
yarn typecheckpassesyarn lintpassesyarn testpasses (251 tests, 12 suites)Closes #3144
🤖 Generated with Claude Code
Summary by CodeRabbit