fix(horizon): initialize Billing with Activity to prevent Null Exception#27
Conversation
WalkthroughHorizon Meta Platform SDK version updated from 72 to 77.0.1 in build config. OpenIapModule.kt: replaced Android Log with OpenIapLog, deferred BillingClient construction to initConnection, and added Activity-aware context selection when building the BillingClient. Changes
Sequence DiagramsequenceDiagram
participant Init as Module Init
participant Module as OpenIapModule
participant Activity as Activity (if any)
participant Client as BillingClient
Note over Init,Module: Old flow (pre-change)
Init->>Module: construct module
Module->>Client: build BillingClient immediately (Context)
Module->>Module: log via Android Log
Note over Init,Module: New flow (post-change)
Init->>Module: construct module
Module->>Module: defer BillingClient construction
Module->>Module: initConnection invoked
Module->>Activity: check for Activity
alt Activity present
Module->>Client: build BillingClient with Activity
rect rgb(200,240,200)
Client->>Module: connection started (OpenIapLog)
end
else Activity absent
Module->>Client: build BillingClient with Context
rect rgb(240,220,200)
Client->>Module: connection started (OpenIapLog)
end
end
Client->>Module: lifecycle logs via OpenIapLog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
hyochan
changed the title
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
835-914: Replace remaining android.util.Log calls with OpenIapLog for consistency.The alternative billing methods still use
android.util.Log.e()at lines 835, 845, 872, 882, 900, and 914, while the rest of the file has been migrated toOpenIapLog. This creates inconsistency in logging.Apply these changes for consistency:
} catch (e: Exception) { - Log.e(TAG, "Error checking alternative billing: ${e.message}") + OpenIapLog.e("Error checking alternative billing", e, TAG) cont.resumeWithException(e) }} catch (e: Exception) { - Log.e(TAG, "Error in checkAlternativeBillingAvailability: ${e.message}") + OpenIapLog.e("Error in checkAlternativeBillingAvailability", e, TAG) false }} catch (e: Exception) { - Log.e(TAG, "Error showing alternative billing dialog: ${e.message}") + OpenIapLog.e("Error showing alternative billing dialog", e, TAG) cont.resumeWithException(e) }} catch (e: Exception) { - Log.e(TAG, "Error in showAlternativeBillingInformationDialog: ${e.message}") + OpenIapLog.e("Error in showAlternativeBillingInformationDialog", e, TAG) false }} catch (e: Exception) { - Log.e(TAG, "Error creating alternative billing token: ${e.message}") + OpenIapLog.e("Error creating alternative billing token", e, TAG) cont.resumeWithException(e) }} catch (e: Exception) { - Log.e(TAG, "Error in createAlternativeBillingReportingToken: ${e.message}") + OpenIapLog.e("Error in createAlternativeBillingReportingToken", e, TAG) null }
🧹 Nitpick comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
787-817: Consider failing fast if Activity is unavailable for Horizon SDK.The Activity-aware context selection is a good approach to fix the NullPointerException. However, the current implementation silently falls back to
Contextwhen no Activity is available (line 796), which may still cause the NPE the PR aims to fix.If the Horizon SDK requires an Activity to function properly, consider failing fast instead of falling back:
private fun buildBillingClient() { // CRITICAL: Use Activity if available, otherwise fall back to Context // Horizon SDK needs Activity to properly initialize OVRPlatform with returnComponent val activity = currentActivityRef?.get() ?: fallbackActivity - val contextForBilling: Context = activity ?: context - if (contextForBilling is Activity) { + if (activity != null) { OpenIapLog.d("Building BillingClient with Activity", TAG) } else { - OpenIapLog.w("No Activity available - Horizon SDK will initialize in limited mode", TAG) + OpenIapLog.e("No Activity available - Horizon SDK requires Activity for proper initialization", TAG) + throw OpenIapError.MissingCurrentActivity } + val contextForBilling: Context = activity + val pendingPurchasesParams = com.meta.horizon.billingclient.api.PendingPurchasesParams.newBuilder() .enableOneTimeProducts() .build()Alternatively, if Context fallback is intentionally supported for certain scenarios, document when "limited mode" is acceptable and what functionality is affected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/google/openiap/build.gradle.kts(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-18T05:46:51.596Z
Learnt from: hyochan
PR: hyodotdev/openiap#17
File: packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt:620-629
Timestamp: 2025-10-18T05:46:51.596Z
Learning: In packages/google/openiap/src/main/java/**/*.kt (Android-only package): DO NOT add Android suffix to function names, even for Android-specific APIs. Exception: Only use Android suffix for cross-platform API types (e.g., ProductAndroid, PurchaseAndroid) that contrast with iOS types. Examples of correct naming: isHorizonEnvironment(context: Context), buildModule(context: Context), acknowledgePurchase(), consumePurchase().
Applied to files:
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt
🧬 Code graph analysis (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt (1)
buildBillingClient(904-1023)
⏰ 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: Test Android
🔇 Additional comments (3)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (2)
82-85: LGTM - Logging consolidated.The Android Log calls have been properly replaced with OpenIapLog equivalents for consistency.
113-143: LGTM - Connection initialization improved.The logging has been properly consolidated to OpenIapLog, and the logic correctly rebuilds the BillingClient if it was previously destroyed by
endConnection().packages/google/openiap/build.gradle.kts (1)
93-94: Verify compatibility of SDK version 77.0.1 before merging.The Meta Horizon Platform SDK version bump from 72 to 77.0.1 is a significant jump. No published changelog or breaking changes documentation is available for version 77.0.1, and no security advisories were found for this package.
Since this is being merged to fix a NullPointerException, verify that:
- Version 77.0.1 is compatible with your codebase and does not introduce breaking API changes
- The NPE issue is actually resolved with this version (or has another fix)
- The updated SDK dependencies are tested on the target platforms
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 (3)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (3)
789-823: Consider enforcing Activity requirement or deferring initialization.The KDoc correctly states that Horizon SDK requires Activity and that providing a non-Activity Context causes "limited mode" and "may cause NullPointerException" (lines 792-795). However, the implementation only logs a warning (line 802) and proceeds with non-Activity context.
Given the PR objective is to "prevent Null Exception," consider one of these approaches:
- Enforce Activity requirement: Throw an exception or return early if
contextForBilling !is Activity- Defer initialization: Store the intent to initialize and retry when Activity becomes available via
setActivity()- Queue connection attempts: Block
initConnection()until Activity is set, with a clear error messageThe current approach documents the risk but doesn't prevent it.
Apply this diff to enforce Activity requirement:
private fun buildBillingClient(contextForBilling: Context) { - if (contextForBilling is Activity) { - OpenIapLog.d("Building BillingClient with Activity", TAG) - } else { - OpenIapLog.w("Building BillingClient with Context (not Activity) - Horizon SDK will run in limited mode", TAG) - } + if (contextForBilling !is Activity) { + OpenIapLog.e("FATAL: Horizon SDK requires Activity context. BillingClient initialization aborted.", tag = TAG) + throw OpenIapError.MissingCurrentActivity + } + OpenIapLog.d("Building BillingClient with Activity", TAG)
360-360: Replace remainingandroid.util.Logcall withOpenIapLogfor consistency.Line 360 still uses
android.util.Log.iwhile the rest of the file has been migrated toOpenIapLog. This inconsistency undermines the centralized logging strategy.Apply this diff:
- android.util.Log.i(TAG, "BILLING_FLOW_PARAM: SKU=${productDetails.productId}, resolvedOfferToken=$resolved") + OpenIapLog.i("BILLING_FLOW_PARAM: SKU=${productDetails.productId}, resolvedOfferToken=$resolved", TAG)
826-923: Complete logging migration in alternative billing methods.Lines 841, 851, 878, 888, 906, and 920 still use
Log.einstead ofOpenIapLog.e. The rest of the file has been migrated toOpenIapLog, but these alternative billing methods were missed.Apply these diffs to complete the migration:
} catch (e: Exception) { - Log.e(TAG, "Error checking alternative billing: ${e.message}") + OpenIapLog.e("Error checking alternative billing: ${e.message}", e, TAG) cont.resumeWithException(e)} catch (e: Exception) { - Log.e(TAG, "Error in checkAlternativeBillingAvailability: ${e.message}") + OpenIapLog.e("Error in checkAlternativeBillingAvailability: ${e.message}", e, TAG) false} catch (e: Exception) { - Log.e(TAG, "Error showing alternative billing dialog: ${e.message}") + OpenIapLog.e("Error showing alternative billing dialog: ${e.message}", e, TAG) cont.resumeWithException(e)} catch (e: Exception) { - Log.e(TAG, "Error in showAlternativeBillingInformationDialog: ${e.message}") + OpenIapLog.e("Error in showAlternativeBillingInformationDialog: ${e.message}", e, TAG) false} catch (e: Exception) { - Log.e(TAG, "Error creating alternative billing token: ${e.message}") + OpenIapLog.e("Error creating alternative billing token: ${e.message}", e, TAG) cont.resumeWithException(e)} catch (e: Exception) { - Log.e(TAG, "Error in createAlternativeBillingReportingToken: ${e.message}") + OpenIapLog.e("Error in createAlternativeBillingReportingToken: ${e.message}", e, TAG) null
♻️ Duplicate comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
101-104: Race condition still not resolved—Activity may be unavailable wheninitConnection()is called.While deferring BillingClient construction is correct, the race condition identified in the previous review persists. In React Native,
contextis aReactApplicationContext(Application, not Activity), sofallbackActivity(line 94) is null. WheninitConnection()is invoked beforesetActivity(), line 118 falls back tocontext—an Application context—which Horizon SDK requires to be an Activity (line 792-794 documentation confirms this).This means the NPE that this PR aims to fix (per PR objectives) can still occur if the initialization sequence is:
new OpenIapModule()→initConnection()→setActivity().Recommendation: Add a guard in
initConnection()to defer or reject connection attempts when Activity is unavailable, or register anActivityEventListenerin the React Native bridge to capture Activity immediately upon availability.
🧹 Nitpick comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
5-5: Remove unusedandroid.util.Logimport after completing migration.Once all
Log.eandLog.icalls are replaced withOpenIapLog(see previous comments), this import should be removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-18T05:46:51.596Z
Learnt from: hyochan
PR: hyodotdev/openiap#17
File: packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt:620-629
Timestamp: 2025-10-18T05:46:51.596Z
Learning: In packages/google/openiap/src/main/java/**/*.kt (Android-only package): DO NOT add Android suffix to function names, even for Android-specific APIs. Exception: Only use Android suffix for cross-platform API types (e.g., ProductAndroid, PurchaseAndroid) that contrast with iOS types. Examples of correct naming: isHorizonEnvironment(context: Context), buildModule(context: Context), acknowledgePurchase(), consumePurchase().
Applied to files:
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt
🧬 Code graph analysis (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt (1)
buildBillingClient(904-1023)
⏰ 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: Test Android
🔇 Additional comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
113-141: Logging migration looks good.The consistent use of
OpenIapLogimproves debugging and aligns with the codebase's logging strategy.
1 participant
Resolve meta-quest/Meta-Spatial-SDK-Samples#82 (comment)
Summary by CodeRabbit
Chores
Refactor