Skip to content

[In-app Purchases] Improve error reporting #8113

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 15, 2022
Merged

[In-app Purchases] Improve error reporting #8113

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2da8d3
Show an alert in the IAP debug screen when a purchase fails
koke Nov 11, 2022
7d0aaa4
Improve error handling in the IAP store
koke Nov 11, 2022
5d96d90
Add descriptions for IAP Store errors
koke Nov 14, 2022
8494efb
Remove errors related to other PurchaseResult cases
koke Nov 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion WooCommerce/Classes/ViewRelated/InAppPurchases/InAppPurchasesDebugView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ struct InAppPurchasesDebugView: View {
@State var entitledProductIDs: Set<String> = []
@State var inAppPurchasesAreSupported = true
@State var isPurchasing = false
@State private var purchaseError: PurchaseError? {
didSet {
presentAlert = purchaseError != nil
}
}
@State var presentAlert = false

var body: some View {
List {
Expand All @@ -30,11 +36,16 @@ struct InAppPurchasesDebugView: View {
Button(entitledProductIDs.contains(product.id) ? "Entitled: \(product.description)" : product.description) {
Task {
isPurchasing = true
try? await inAppPurchasesForWPComPlansManager.purchaseProduct(with: product.id, for: siteID)
do {
try await inAppPurchasesForWPComPlansManager.purchaseProduct(with: product.id, for: siteID)
} catch {
purchaseError = PurchaseError(error: error)
}
await loadUserEntitlements()
isPurchasing = false
}
}
.alert(isPresented: $presentAlert, error: purchaseError, actions: {})
}
}
}
Expand Down Expand Up @@ -102,6 +113,20 @@ struct InAppPurchasesDebugView: View {
}
}

/// Just a silly little wrapper because SwiftUI's `alert(isPresented:error:actions:)` wants a `LocalizedError`
/// but we only have an `Error` coming from `purchaseProduct`.
private struct PurchaseError: LocalizedError {
let error: Error

var errorDescription: String? {
if let error = error as? LocalizedError {
return error.errorDescription
} else {
return error.localizedDescription
}
}
}

struct InAppPurchasesDebugView_Previews: PreviewProvider {
static var previews: some View {
InAppPurchasesDebugView(siteID: 0)
Expand Down
105 changes: 99 additions & 6 deletions Yosemite/Yosemite/Stores/InAppPurchaseStore.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,26 @@ private extension InAppPurchaseStore {

logInfo("Purchasing product \(product.id) for site \(siteID) with options \(purchaseOptions)")
let purchaseResult = try await product.purchase(options: purchaseOptions)
if case .success(let result) = purchaseResult {
logInfo("Purchased product \(product.id) for site \(siteID): \(result)")
try await handleCompletedTransaction(result)
} else {
logError("Ignorning unsuccessful purchase: \(purchaseResult)")
switch purchaseResult {
case .success(let result):
guard case .verified(let transaction) = result else {
// Ignore unverified transactions.
logError("Transaction unverified: \(result)")
throw Errors.unverifiedTransaction
}
logInfo("Purchased product \(product.id) for site \(siteID): \(transaction)")

try await submitTransaction(transaction)
await transaction.finish()
case .userCancelled:
logInfo("User cancelled the purchase flow")
throw Errors.userCancelled
Copy link
Contributor

@toupper toupper Nov 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but userCancelled or pending doesn't look exactly as errors to me, more like a "result". We could mimic Apple behavior and instead of throw an error, return an enum PurchaseResult (or typealias?):

public enum PurchaseResult {

        /// The purchase succeeded with a `Transaction`.
        case success(VerificationResult<Transaction>)

        /// The user cancelled the purchase.
        case userCancelled

        /// The purchase is pending some user action.
        ///
        /// These purchases may succeed in the future, and the resulting `Transaction` will be
        /// delivered via `Transaction.updates`
        case pending
    }

However, I understand that throwing an error is easier to implement and probably to handle, so if you prefer to leave as it is it is fine for me as well.

Copy link
Member Author

@koke koke Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Actually, purchaseProduct is already returning a PurchaseResult but I ended up turning those two cases into errors 🤦🏽

Copy link
Member Author

@koke koke Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed those errors in 8494efb and tested that cancelling a purchase doesn't show an alert, while a network error during purchase shows the alert.

case .pending:
logError("Purchase returned in a pending state, it might succeed in the future")
throw Errors.pending
@unknown default:
logError("Unknown result for purchase: \(purchaseResult)")
throw Errors.unknownResult
}
completion(.success(purchaseResult))
} catch {
Expand Down Expand Up @@ -265,13 +280,91 @@ private extension InAppPurchaseStore {
}

public extension InAppPurchaseStore {
enum Errors: Error {
enum Errors: Error, LocalizedError {
/// The user canceled the IAP flow
case userCancelled

/// The purchase is pending some user action.
///
/// These purchases may succeed in the future, and the resulting `Transaction` will be
/// delivered via `Transaction.updates`
case pending

/// The purchase returned a PurchaseResult value that didn't exist when this was developed
case unknownResult

/// The purchase was successful but the transaction was unverified
///
case unverifiedTransaction

/// The purchase was successful but it's not associated to an account
///
case transactionMissingAppAccountToken

/// The transaction has an associated account but it can't be translated to a site
///
case appAccountTokenMissingSiteIdentifier

/// The transaction is associated with an unknown product
///
case transactionProductUnknown

/// The storefront for the user is unknown, and so we can't know their country code
///
case storefrontUnknown

/// App receipt was missing, even after a refresh
///
case missingAppReceipt

/// In-app purchases are not supported for this user
///
case inAppPurchasesNotSupported

public var errorDescription: String? {
switch self {
case .userCancelled:
return NSLocalizedString(
"Purchase cancelled by user",
comment: "Error message used when the user cancelled an In-app purchase flow")
case .pending:
return NSLocalizedString(
"Purchase pending",
comment: "Error message used when the purchase is pending some user action")
case .unknownResult:
return NSLocalizedString(
"Unexpected purchase result",
comment: "Error message used when a purchase returned something unexpected that we don't know how to handle")
case .unverifiedTransaction:
return NSLocalizedString(
"The purchase transaction couldn't be verified",
comment: "Error message used when a purchase was successful but its transaction was unverified")
case .transactionMissingAppAccountToken:
return NSLocalizedString(
"Purchase transaction missing account information",
comment: "Error message used when the purchase transaction doesn't have the right metadata to associate to a specific site")
case .appAccountTokenMissingSiteIdentifier:
return NSLocalizedString(
"Purchase transaction can't be associated to a site",
comment: "Error message used when the purchase transaction doesn't have the right metadata to associate to a specific site")
case .transactionProductUnknown:
return NSLocalizedString(
"Purchase transaction received for an unknown product",
comment: "Error message used when we received a transaction for an unknown product")
case .storefrontUnknown:
return NSLocalizedString(
"Couldn't determine App Stoure country",
comment: "Error message used when we can't determine the user's App Store country")
case .missingAppReceipt:
return NSLocalizedString(
"Couldn't retrieve app receipt",
comment: "Error message used when we can't read the app receipt")
case .inAppPurchasesNotSupported:
return NSLocalizedString(
"In-app purchases are not supported for this user yet",
comment: "Error message used when In-app purchases are not supported for this user/site")
}
}
}

enum Constants {
Expand Down