New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[expo-in-app-purchases] Add defensive null checks and missing iOS SKErrorCode values #11773
Conversation
.../expo-in-app-purchases/android/src/main/java/expo/modules/inapppurchases/BillingManager.java
Outdated
Show resolved
Hide resolved
packages/expo-in-app-purchases/ios/EXInAppPurchases/EXInAppPurchasesModule.m
Outdated
Show resolved
Hide resolved
packages/expo-in-app-purchases/ios/EXInAppPurchases/EXInAppPurchasesModule.m
Outdated
Show resolved
Hide resolved
packages/expo-in-app-purchases/ios/EXInAppPurchases/EXInAppPurchasesModule.m
Outdated
Show resolved
Hide resolved
return 13; | ||
case SKErrorMissingOfferParams: | ||
case SKErrorUnsupportedPlatform: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know what triggers this? Is it e.g. Catalyst code on macOS for an app that supports IAPs only on iOS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't know what triggers SKErrorUnsupportedPlatform
. The Apple docs are empty (https://developer.apple.com/documentation/storekit/skerrorcode/skerrorunsupportedplatform).
I will reassign this to 0 (UNKNOWN) for now. (I've also commented the magic numbers to show which IAPErrorCode enum value they correspond to).
- BillingManager.java: Formatting updates - EXInAppPurchasesModule.m: Formatting updates and IAPErrorCode re-assignments
This is on npm as version 10.1.0. |
Thank you for the speedy review! |
Why
We have seen several crash reports in production on Android and iOS due to the platform IAP code sending invalid or missing Product SKU data to
expo-in-app-purchases
.On Android, despite the documentation implying that the parameters to methods like
onQueryPurchasesFinished()
andonSkuDetailsResponse()
are not nullable, we have seen occasional crashes in production where they have been null. This patch adds some defensive null checks to the Android code to gracefully ignore invalid input rather than crash.On iOS, there are cases when some fields of
SKProduct
coming from the StoreKit API can benil
, specifically during the app review process when your IAP product is in a "Review Rejected" state. In this case, I think it's best to ignore the invalid product SKU, otherwise thenil
values will propagate through the EXInAppPurchases code and inevitably cause a native Objective-C crash.(example crash report: https://sentry.io/share/issue/057be1d8c2f345c687feb82349a87adb/ - note, I believe the part of the stack trace that includes
expo::gl_cpp
is bogus due to threading or JSC issues - the actual error comes from receiving a SKProduct containingnil
values from[SKProductsRequest _start]
and passing it togetProductData
)In addition, this patch adds a few missing
SKErrorCode
values toerrorCodeNativeToJS
that were added in more recent StoreKit versions. (happy to separate this part if you'd like).By the way, separately, I'd like to propose updating the Android Google Play Billing SDK to 2.2.0, which is the latest version that has no breaking changes relative to the version currently used by
expo-in-app-purchases
(2.0.0).How
Ignore invalid product SKUs at the Android/iOS API level.
Test Plan
We've been running this patch in production on our app (https://badukpop.com) for several months with satisfactory results.