Skip to content

Commit

Permalink
[expo-in-app-purchases] Add defensive null checks and missing iOS SKE…
Browse files Browse the repository at this point in the history
…rrorCode values (#11773)

# 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()` and `onSkuDetailsResponse()` 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 be `nil`, 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 the `nil` 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 containing `nil` values from `[SKProductsRequest _start]` and passing it to `getProductData`)

In addition, this patch adds a few missing `SKErrorCode` values to `errorCodeNativeToJS` 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.
  • Loading branch information
danmaas committed Feb 2, 2021
1 parent 0e08d74 commit b8dc87d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public Context getContext() {
*/
@Override
public void onPurchasesUpdated(BillingResult result, List<Purchase> purchases) {
if (result.getResponseCode() == BillingResponseCode.OK) {
if (result.getResponseCode() == BillingResponseCode.OK && purchases != null) {
for (Purchase purchase : purchases) {
handlePurchase(purchase);
}
Expand Down Expand Up @@ -263,11 +263,11 @@ public void run() {
PurchasesResult purchasesResult = mBillingClient.queryPurchases(SkuType.INAPP);

// If there are subscriptions supported, we add subscription rows as well
if (areSubscriptionsSupported()) {
if (purchasesResult != null && areSubscriptionsSupported()) {
PurchasesResult subscriptionResult
= mBillingClient.queryPurchases(SkuType.SUBS);

if (subscriptionResult.getResponseCode() == BillingResponseCode.OK) {
if (subscriptionResult != null && subscriptionResult.getResponseCode() == BillingResponseCode.OK) {
purchasesResult.getPurchasesList().addAll(
subscriptionResult.getPurchasesList());
}
Expand Down Expand Up @@ -441,7 +441,7 @@ private static Bundle purchaseHistoryToBundle(PurchaseHistoryRecord purchaseReco
*/
private void onQueryPurchasesFinished(PurchasesResult result, final Promise promise) {
// Have we been disposed of in the meantime? If so, or bad result code, then quit
if (mBillingClient == null || result.getResponseCode() != BillingResponseCode.OK) {
if (mBillingClient == null || result == null || result.getResponseCode() != BillingResponseCode.OK) {
promise.reject("E_QUERY_FAILED", "Billing client was null or query was unsuccessful");
return;
}
Expand Down Expand Up @@ -481,7 +481,7 @@ public void onSkuDetailsResponse(BillingResult inAppResult,
mBillingClient.querySkuDetailsAsync(subs.build(), new SkuDetailsResponseListener() {
@Override
public void onSkuDetailsResponse(BillingResult billingResult, List<SkuDetails> subscriptionDetails) {
if (skuDetails != null) {
if (skuDetails != null && subscriptionDetails != null) {
skuDetails.addAll(subscriptionDetails);
}
listener.onSkuDetailsResponse(billingResult, skuDetails);
Expand All @@ -501,9 +501,11 @@ public void queryPurchasableItems(List<String> itemList, final Promise promise)
@Override
public void onSkuDetailsResponse(BillingResult billingResult, List<SkuDetails> skuDetailsList) {
ArrayList<Bundle> results = new ArrayList<>();
for (SkuDetails skuDetails : skuDetailsList) {
mSkuDetailsMap.put(skuDetails.getSku(), skuDetails);
results.add(skuToBundle(skuDetails));
if (skuDetailsList != null) {
for (SkuDetails skuDetails : skuDetailsList) {
mSkuDetailsMap.put(skuDetails.getSku(), skuDetails);
results.add(skuToBundle(skuDetails));
}
}
Bundle response = formatResponse(billingResult, results);
promise.resolve(response);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ - (void)handleQuery:(SKProductsResponse *)response {
NSMutableArray *result = [NSMutableArray array];

for (SKProduct *validProduct in response.products) {
if (!validProduct.localizedDescription) { continue; } // skip product with nil values - this can happen if it is in review "rejected" state
NSDictionary *productData = [self getProductData:validProduct];
[result addObject:productData];
}
Expand All @@ -167,6 +168,7 @@ -(void)handlePurchase:(SKProductsResponse *)response {
}

for (SKProduct *validProduct in response.products) {
if (!validProduct.localizedDescription) { continue; } // skip product with nil values - this can happen if it is in review "rejected" state
[self purchase:validProduct];
}
}
Expand Down Expand Up @@ -412,35 +414,41 @@ - (NSDictionary *)formatResults:(SKErrorCode)errorCode
};
}

// Convert native error code to match TS enum
// Convert native error code to match TS enum IAPErrorCode
- (int)errorCodeNativeToJS:(SKErrorCode)errorCode
{
switch(errorCode) {
case SKErrorUnknown:
return 0;
case SKErrorUnsupportedPlatform:
return 0; // UNKNOWN
case SKErrorClientInvalid:
case SKErrorPaymentInvalid:
case SKErrorPaymentNotAllowed:
case SKErrorPaymentCancelled:
return 1;
case SKErrorOverlayCancelled:
return 1; // PAYMENT_INVALID
case SKErrorOverlayTimeout:
return 4; // SERVICE_TIMEOUT
case SKErrorStoreProductNotAvailable:
return 6;
return 6; // ITEM_UNAVAILABLE
case SKErrorCloudServiceRevoked:
case SKErrorCloudServicePermissionDenied:
case SKErrorCloudServiceNetworkConnectionFailed:
return 10;
return 10; // CLOUD_SERVICE
case SKErrorPrivacyAcknowledgementRequired:
return 11;
return 11; // PRIVACY_UNACKNOWLEDGED
case SKErrorUnauthorizedRequestData:
return 12;
return 12; // UNAUTHORIZED_REQUEST
case SKErrorInvalidSignature:
case SKErrorInvalidOfferPrice:
case SKErrorInvalidOfferIdentifier:
return 13;
case SKErrorOverlayInvalidConfiguration:
case SKErrorIneligibleForOffer:
return 13; // INVALID_IDENTIFIER
case SKErrorMissingOfferParams:
return 14;
return 14; // MISSING_PARAMS
default:
return 0;
return 0; // UNKNOWN
}
}

Expand Down

0 comments on commit b8dc87d

Please sign in to comment.