Skip to content

ReceiptFetcher: refactored implementation to log error when failing to fetch receipt#2202

Merged
NachoSoto merged 2 commits into
mainfrom
receipt-fetcher-error
Jan 16, 2023
Merged

ReceiptFetcher: refactored implementation to log error when failing to fetch receipt#2202
NachoSoto merged 2 commits into
mainfrom
receipt-fetcher-error

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Changes:

  • Added Optional.orThrow to simplify implementation (this will be useful in many other parts of the code too)
  • Added missing OptinalExtensionsTests
    DefaultFileReader was implemented using try?, now it throws an error instead of returning nil
  • ReceiptFetcher: logging specific error when loading receipt fails
  • Refactored ReceiptFetcher:
    • No longer logging "loaded receipt" if the content was empty
    • No longer logging an error twice if the receipt failed to load
    • Simplified logic to handle errors only once

@NachoSoto NachoSoto requested a review from a team January 10, 2023 21:57
NachoSoto added a commit that referenced this pull request Jan 10, 2023
Fixes [CSDK-622].
Depends on #2199, #2200, #2202, #2203.

- Extracted `FileReader` to be able to use it in `ReceiptParser`
- Extracted `MockBundle` to be able to use it in `ReceiptParser`
- Added new `public` `ReceiptParser.fetchAndParseLocalReceipt()`, **only to `ReceiptParser`**. This is not available through `RevenueCat`.
- Added tests for the new API
- Added new API to `ReceiptParserAPITester`

Initially I thought about moving `ReceiptFetcher` to the `Receipt|Parser` target. Unfortunately that would require moving a whole bunch of other dependencies (`OperationDispatcher`, etc, etc).
We have no easy way to create a third "Core" framework in which to place these common dependencies, so that wouldn't be trivial.

This solution adds a much simpler version of `ReceiptFetcher` that simply reads from `Bundle.appStoreReceiptURL`. As it's documented there, under normal circumstances `SKReceiptRefreshRequest` is not required, so this implementation should work for 99% of use cases, and so this way we avoid increasing the.codebase complexity unnecessarily.
NachoSoto added a commit that referenced this pull request Jan 12, 2023
Fixes [CSDK-622].
Depends on #2199, #2200, #2202, #2203.

- Extracted `FileReader` to be able to use it in `ReceiptParser`
- Extracted `MockBundle` to be able to use it in `ReceiptParser`
- Added new `public` `ReceiptParser.fetchAndParseLocalReceipt()`, **only to `ReceiptParser`**. This is not available through `RevenueCat`.
- Added tests for the new API
- Added new API to `ReceiptParserAPITester`

Initially I thought about moving `ReceiptFetcher` to the `Receipt|Parser` target. Unfortunately that would require moving a whole bunch of other dependencies (`OperationDispatcher`, etc, etc).
We have no easy way to create a third "Core" framework in which to place these common dependencies, so that wouldn't be trivial.

This solution adds a much simpler version of `ReceiptFetcher` that simply reads from `Bundle.appStoreReceiptURL`. As it's documented there, under normal circumstances `SKReceiptRefreshRequest` is not required, so this implementation should work for 99% of use cases, and so this way we avoid increasing the.codebase complexity unnecessarily.
@NachoSoto NachoSoto force-pushed the receipt-fetcher-error branch from a3c1f6a to 1084959 Compare January 12, 2023 18:28
… to fetch receipt

- Added `Optional.orThrow` to simplify implementation (this will be useful in many other parts of the code too)
- Added missing `OptinalExtensionsTests`
`DefaultFileReader` was implemented using `try?`, now it throws an error instead of returning `nil`
- `ReceiptFetcher`: logging specific error when loading receipt fails
- Refactored `ReceiptFetcher`:
  - No longer logging "loaded receipt" if the content was empty
  - No longer logging an error twice if the receipt failed to load
  - Simplified logic to handle errors only once
@NachoSoto NachoSoto force-pushed the receipt-fetcher-error branch from 1084959 to 4b1a85b Compare January 12, 2023 20:04

@tonidero tonidero left a comment

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.

LGTM!

@NachoSoto NachoSoto merged commit e4bbd05 into main Jan 16, 2023
@NachoSoto NachoSoto deleted the receipt-fetcher-error branch January 16, 2023 15:43
NachoSoto pushed a commit that referenced this pull request Jan 17, 2023
**This is an automatic release.**

### New Features
* Added new `ReceiptParser.fetchAndParseLocalReceipt` (#2204) via
NachoSoto (@NachoSoto)
* `PurchasesReceiptParser`: added API to parse receipts from `base64`
string (#2200) via NachoSoto (@NachoSoto)
### Bugfixes
* `CustomerInfo`: support parsing schema version 2 to restore SDK `v3.x`
compatibility (#2213) via NachoSoto (@NachoSoto)
### Other Changes
* `JSONDecoder`: added decoding type when logging
`DecodingError.keyNotFound` (#2212) via NachoSoto (@NachoSoto)
* Added `ReceiptParserTests` (#2203) via NachoSoto (@NachoSoto)
* Deploy `PurchaseTester` for `macOS` (#2011) via NachoSoto (@NachoSoto)
* `ReceiptFetcher`: refactored implementation to log error when failing
to fetch receipt (#2202) via NachoSoto (@NachoSoto)
* `PostReceiptDataOperation`: replaced receipt `base64` with `hash` for
cache key (#2199) via NachoSoto (@NachoSoto)
* `PurchaseTester`: small refactor to simplify `Date` formatting (#2210)
via NachoSoto (@NachoSoto)
* `PurchasesReceiptParser`: improved documentation to reference
`default` (#2197) via NachoSoto (@NachoSoto)
* Created `CachingTrialOrIntroPriceEligibilityChecker` (#2007) via
NachoSoto (@NachoSoto)
* Update Gemfile.lock (#2205) via Cesar de la Vega (@vegaro)
* remove stalebot in favor of SLAs in Zendesk (#2196) via Andy Boedo
(@aboedo)
* Update fastlane-plugin-revenuecat_internal to latest version (#2194)
via Cesar de la Vega (@vegaro)
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