-
Notifications
You must be signed in to change notification settings - Fork 121
[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.
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
Conversation
You can test the changes from this Pull Request by:
|
|
The code looks great (just a non-blocker comment) and tests well when the user cancels the purchase. Thanks for that! Now we are able to communicate better what happened to the IAP interface client. 🎉 |
| await transaction.finish() | ||
| case .userCancelled: | ||
| logInfo("User cancelled the purchase flow") | ||
| throw Errors.userCancelled |
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.
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.
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.
Good point. Actually, purchaseProduct is already returning a PurchaseResult but I ended up turning those two cases into errors 🤦🏽
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.
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.
Closes: #8112
Description
This PR tries to make the IAP logging clearer and shows any error purchasing as an alert in the debug view. This includes:
submitTransactiondirectly if the purchase was successful and verified. We don't reusehandleCompletedTransactionanymore and that method is reserved for the transaction listener. This means different logging for the interactive purchase and transaction listener so that hopefully we'll differentiate those better in the logs.Testing instructions
There are many different ways to test all the errors, but the simplest way to verify this:
Screenshots
RELEASE-NOTES.txtif necessary.