Skip to content

Added BackendErrorCode.purchasedProductMissingInAppleReceipt#2033

Merged
NachoSoto merged 2 commits into
mainfrom
backend-error-missing-product-receipt
Nov 8, 2022
Merged

Added BackendErrorCode.purchasedProductMissingInAppleReceipt#2033
NachoSoto merged 2 commits into
mainfrom
backend-error-missing-product-receipt

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

See https://github.com/RevenueCat/khepri/pull/4716
This makes that error ErrorCode.invalidReceiptError instead of .unknownBackendError.

I've also added #2032 to improve error messages when these errors are unknown in the future.

See https://github.com/RevenueCat/khepri/pull/4716
This makes that error `ErrorCode.invalidReceiptError` instead of `.unknownBackendError`.

I've also added #2032 to improve error messages when these errors are unknown in the future.
@NachoSoto NachoSoto requested review from a team and antoniobg November 7, 2022 22:08
case .cannotTransferPurchase:
return .receiptAlreadyInUseError
case .invalidReceiptToken:
case .invalidReceiptToken, .purchasedProductMissingInAppleReceipt:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick, these multi-cases seem to be set in multiple lines here. Just to be consistent:

Suggested change
case .invalidReceiptToken, .purchasedProductMissingInAppleReceipt:
case .invalidReceiptToken,
.purchasedProductMissingInAppleReceipt:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, not sure if we have tests for these... If we do, it might be good to add one, if not, I guess something to think about.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What type of test would you write, other than simply duplicating this logic?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I think it would be good to have a test that checks the mapping from backend code to purchases code for each of these values. As you said, it's basically duplicating the logic, so not a huge benefit, but might catch some issues if someone changes the mapping at some point and moves one of the error codes accidentally. That said, I don't think this is needed for this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Like you said yourself, that would just be duplicating the logic. This is purely data-driven, so the data is the test as well.

for each of these values.

In order to do this cleanly, we would need to have a mapping for each of the errors, instead of duplicating each test. Then iterate the mapping and assert it's correct... but wait, that mapping is the data we're testing, so there would be no valid in that duplication.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well I agree that it's redundant, but there is some security in redundancy. In any case, I agree the chances of this being changed mistakenly are minimal and if we did this, we would probably have to duplicate logic for tests in a bunch of places so I'm ok with leaving this.

@NachoSoto NachoSoto merged commit a69cb94 into main Nov 8, 2022
@NachoSoto NachoSoto deleted the backend-error-missing-product-receipt branch November 8, 2022 17:14
NachoSoto pushed a commit that referenced this pull request Nov 9, 2022
**This is an automatic release.**

### Bugfixes
* `ISO8601DateFormatter.withMilliseconds`: fixed iOS 11 crash (#2037)
via NachoSoto (@NachoSoto)
* Changed `StoreKit2Setting.default` back to
`.enabledOnlyForOptimizations` (#2022) via NachoSoto (@NachoSoto)
### Other Changes
* `Integration Tests`: changed weekly to monthly subscriptions to work
around 0-second subscriptions (#2042) via NachoSoto (@NachoSoto)
* `Integration Tests`: fixed `testPurchaseWithAskToBuyPostsReceipt`
(#2040) via NachoSoto (@NachoSoto)
* `ReceiptRefreshPolicy.retryUntilProductIsFound`: default to returning
"invalid" receipt (#2024) via NachoSoto (@NachoSoto)
* `CachingProductsManager`: use partial cached products (#2014) via
NachoSoto (@NachoSoto)
* Added `BackendErrorCode.purchasedProductMissingInAppleReceipt` (#2033)
via NachoSoto (@NachoSoto)
* `PurchaseTesterSwiftUI`: replaced `Purchases` dependency with `SPM`
(#2027) via NachoSoto (@NachoSoto)
* `Integration Tests`: changed log output to `raw` (#2031) via NachoSoto
(@NachoSoto)
* `Integration Tests`: run on iOS 16 (#2035) via NachoSoto (@NachoSoto)
* CI: fixed `iOS 14` tests Xcode version (#2030) via NachoSoto
(@NachoSoto)
* `Async.call`: added non-throwing overload (#2006) via NachoSoto
(@NachoSoto)
* Documentation: Fixed references in `V4_API_Migration_guide.md` (#2018)
via NachoSoto (@NachoSoto)
* `eligiblePromotionalOffers`: don't log error if response is ineligible
(#2019) via NachoSoto (@NachoSoto)
* Runs push-pods after make-release (#2025) via Cesar de la Vega
(@vegaro)
* Some updates on notify-on-non-patch-release-branches: (#2026) via
Cesar de la Vega (@vegaro)
* Deploy `PurchaseTesterSwiftUI` to TestFlight (#2003) via NachoSoto
(@NachoSoto)
* `PurchaseTesterSwiftUI`: added "logs" screen (#2012) via NachoSoto
(@NachoSoto)
* `PurchaseTesterSwiftUI`: allow configuring API key at runtime (#1999)
via NachoSoto (@NachoSoto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants