`StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2` by NachoSoto · Pull Request #1910 · RevenueCat/purchases-ios

NachoSoto · 2022-09-16T17:37:38Z

Fixes CSDK-464. Follow up to #1869.

SK1 and SK2 had inconsistent behavior during cancelled purchases.

Before this PR:

SK1 / completion-block: no CustomerInfo + cancelled + ErrorCode.purchaseCancelledError
SK1 / async: thrown ErrorCode.purchaseCancelledError ⚠️
SK2 / completion-block: CustomerInfo + cancelled + ErrorCode.purchaseCancelledError
SK2 / async: CustomerInfo + cancelled (not error)

After this PR:

The output is now consistent across the board:

SK1 / completion-block: CustomerInfo + cancelled + ErrorCode.purchaseCancelledError
SK1 / async: CustomerInfo + cancelled (not error)
SK2 / completion-block: CustomerInfo + cancelled + ErrorCode.purchaseCancelledError
SK2 / async: CustomerInfo + cancelled (not error)

Notes

This is of course a somewhat backwards incompatible change, but this output is now what makes the most sense based on the types. This would only break existing code if users are relying on the existence of CustomerInfo alone without checking error or userCancelled.

Completion-block API:

There's 2 ways of detecting cancellation, both valid and reasonable. Either checking that cancelled is true, or comparing error with ErrorCode.purchaseCancelledError

async API:

A cancelled purchase no longer throws. This didn't make sense in SK1, because it meant that userCancelled was always false in the case that the purchase method returned. This also matches StoreKit's behavior in Product.purchase()

…with StoreKit 2 Fixes [CSDK-464]. Follow up to #1869. SK1 and SK2 had inconsistent behavior during cancelled purchases. - SK1 / completion-block: no `CustomerInfo` + cancelled + `ErrorCode.purchaseCancelledError` - **SK1 / `async`: thrown `ErrorCode.purchaseCancelledError`** ⚠️ - SK2 / completion-block: `CustomerInfo` + cancelled + `ErrorCode.purchaseCancelledError` - SK2 / `async`: `CustomerInfo` + cancelled (not error) The output is now consistent across the board: - SK1 / completion-block: `CustomerInfo` + cancelled + `ErrorCode.purchaseCancelledError` - SK1 / `async`: `CustomerInfo` + cancelled (not error) - SK2 / completion-block: `CustomerInfo` + cancelled + `ErrorCode.purchaseCancelledError` - SK2 / `async`: `CustomerInfo` + cancelled (not error) This is of course a somewhat backwards incompatible change, but this output is now what makes the most sense based on the types. *This would only break existing code if users are relying on the existence of `CustomerInfo` alone without checking `error` or `userCancelled`. - Completion-block API: There's 2 ways of detecting cancellation, both valid and reasonable. Either checking that `cancelled` is `true`, or comparing `error` with `ErrorCode.purchaseCancelledError` - `async` API: A cancelled purchase no longer throws. This didn't make sense in SK1, because it meant that `userCancelled` was always `false` in the case that the purchase method returned.

tonidero

I can see how this changes the behavior and users could potentially be relying in the old behavior... While I wouldn't consider it a breaking change, we should mention this prominently in the Changelog on the next release.

tonidero · 2022-09-22T11:14:19Z

-                )
+                result = try await self.purchases.purchase(product: product)
            } catch {
                receivedError = error as NSError


Could we replace this with a fail?

No because it's inside of a Task so it's happening asynchronously.
There's a comment above explaining why it has to be this way:

// Need to do this async so the code below can invoke the updatedTransaction delegate method.

Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>

NachoSoto · 2022-09-22T23:28:17Z

we should mention this prominently in the Changelog on the next release.

💯

@NachoSoto

**This is an automatic release.** ### New Features * 🚨 `StoreKit 2` is now enabled by default 🚨 (#1922) via NachoSoto (@NachoSoto) * Extracted `PurchasesType` and `PurchasesSwiftType` (#1912) via NachoSoto (@NachoSoto) ### Bugfixes * `StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2` (#1910) via NachoSoto (@NachoSoto) * `PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled (#1901) via NachoSoto (@NachoSoto) ### Other Changes * Fixed iOS 12 tests (#1936) via NachoSoto (@NachoSoto) * `CacheableNetworkOperation`: fixed race condition in new test (#1932) via NachoSoto (@NachoSoto) * `BasePurchasesTests`: changed default back to SK1 (#1935) via NachoSoto (@NachoSoto) * `Logger`: refactored default `LogLevel` definition (#1934) via NachoSoto (@NachoSoto) * `AppleReceipt`: refactored declarations into nested types (#1933) via NachoSoto (@NachoSoto) * `Integration Tests`: relaunch tests when retrying failures (#1925) via NachoSoto (@NachoSoto) * `CircleCI`: downgraded release jobs to Xcode 13.x (#1927) via NachoSoto (@NachoSoto) * `ErrorUtils`: added test to verify that `PublicError`s can be `catch`'d as `ErrorCode` (#1924) via NachoSoto (@NachoSoto) * `StoreKitIntegrationTests`: print `AppleReceipt` data whenever `verifyEntitlementWentThrough` fails (#1929) via NachoSoto (@NachoSoto) * `OperationQueue`: log debug message when requests are found in cache and skipped (#1926) via NachoSoto (@NachoSoto) * `GetCustomerInfoAPI`: avoid making a request if there's any `PostReceiptDataOperation` in progress (#1911) via NachoSoto (@NachoSoto) * `PurchaseTester`: allow HTTP requests and enable setting `ProxyURL` (#1917) via NachoSoto (@NachoSoto)

…PurchasesOrchestrator` for cancelled purchases Fixes SDK-3088 Supersedes #2447 Note that it's currently impossible to test this for SK2, so we only have SK1 tests (we have a Radar for that). The next step will be to deprecate the current API to remove the `userCancelled` property and make this the new behavior. See also #1910 for an explanation of the different return values of the purchase APIs.

@tonidero

…cancelled purchases (#2449) Fixes SDK-3088 Supersedes #2447 Thanks to @tonidero for doing most of this. Note that it's currently impossible to test this for SK2, so we only have SK1 tests (we have a Radar for that). The next step will be to deprecate the current API to remove the `userCancelled` property and make this the new behavior. See also #1910 for an explanation of the different return values of the purchase APIs.

…d` as `unavailable` Follow up to #2449. This is step 2 of 3 towards getting rid of `userCancelled` in purchase results. After #2449, `ENABLE_CUSTOM_ENTITLEMENT_COMPUTATION` builds no longer send `userCancelled` `true` for cancellations, and instead throw `ErrorCode.purchaseCancelledError`. To avoid users detecting cancellations looking at this property, it's not deprecated. To achieve that, I converted the `tuple` into a `struct`. This also required moving the affected `purchase` methods from `PurchasesType` to `PurchasesSwiftType`, but that's an implementation detail. The public API remains _mostly_ unchanged, with the only caveat that it's no longer possible to destructure the result tuple, so: ```swift let (transaction, customerInfo, cancelled) = Purchases.shared.purchase(...) ``` Needs to become: ```swift let result = Purchases.shared.purchase(...) ``` I updated the API tester to reflect that, as well as checking the properties of `PurchaseResultData`. As it's documented in #1910, completion block APIs already did send both `userCancelled` and the `error`, so we don't need to change those. This paves the future for a full deprecation, where we mark it `deprecated` for normal builds as well, and change the behavior regardless of the `DangerousSettings`.

@tonidero

…cancelled purchases (#2449) Fixes SDK-3088 Supersedes #2447 Thanks to @tonidero for doing most of this. Note that it's currently impossible to test this for SK2, so we only have SK1 tests (we have a Radar for that). The next step will be to deprecate the current API to remove the `userCancelled` property and make this the new behavior. See also #1910 for an explanation of the different return values of the purchase APIs.

NachoSoto added the pr:fix A bug fix label Sep 16, 2022

NachoSoto requested a review from a team September 16, 2022 17:37

NachoSoto force-pushed the sk1-cancellation-output branch from 8573da0 to db62724 Compare September 16, 2022 17:38

NachoSoto force-pushed the sk1-cancellation-output branch from db62724 to 8fcf133 Compare September 16, 2022 17:38

NachoSoto changed the title ~~StoreKit 1: changed result of cancelled purchases to be consistent with StoreKit 2~~ StoreKit 1: changed result of cancelled purchases to be consistent with StoreKit 2 Sep 16, 2022

tonidero approved these changes Sep 22, 2022

View reviewed changes

Renamed variable

bce406c

Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>

NachoSoto merged commit 6d176f0 into main Sep 22, 2022

NachoSoto deleted the sk1-cancellation-output branch September 22, 2022 23:29

NachoSoto mentioned this pull request Sep 23, 2022

[AUTOMATIC] Release/4.13.0 #1938

Merged

NachoSoto mentioned this pull request Sep 28, 2022

"There was a problem with the App Store." error when accessing userCancelled property on purchase when user cancels transaction #1947

Closed

3 tasks

NachoSoto mentioned this pull request Apr 26, 2023

Custom Entitlement Computation: avoid getCustomerInfo requests for cancelled purchases #2449

Merged

NachoSoto mentioned this pull request Apr 26, 2023

Custom Entitlement Computation: mark PurchaseResultData.userCancelled as unavailable #2450

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2`#1910

`StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2`#1910
NachoSoto merged 2 commits into
mainfrom
sk1-cancellation-output

NachoSoto commented Sep 16, 2022 •

edited

Loading

Uh oh!

tonidero left a comment

Uh oh!

Uh oh!

tonidero Sep 22, 2022

Uh oh!

NachoSoto Sep 22, 2022

Uh oh!

NachoSoto commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

NachoSoto commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before this PR:

After this PR:

Notes

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonidero Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto Sep 22, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Sep 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NachoSoto commented Sep 16, 2022 •

edited

Loading