Skip to content
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

ErrorUtils.purchasesError(withSKError:): handle URLErrors #3346

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

NachoSoto
Copy link
Contributor

Fixes #3343.

SKPaymentTransaction.error docs say:

The error property is undefined except when transactionState is set to SKPaymentTransactionState.failed. Your application can read the error property to determine why the transaction failed. For a list of error constants, see SKErrorDomain in StoreKit Constants.

Which is why PurchasesOrchestrator only considered that domain.

This expands this handling to consider any other network error, therefore providing more precise errors.

Since offline connection error detection was only in HTTPClient, I also moved that out and removed NetworkError.offlineConnection, so that we detect this when converting NetworkError.networkError instead.

Fixes #3343.

`SKPaymentTransaction.error` docs say:
> The error property is undefined except when `transactionState` is set to `SKPaymentTransactionState.failed`. Your application can read the error property to determine why the transaction failed. For a list of error constants, see `SKErrorDomain` in StoreKit Constants.

Which is why [`PurchasesOrchestrator` only considered that domain](https://github.com/RevenueCat/purchases-ios/blob/4.28.1/Sources/Purchasing/Purchases/PurchasesOrchestrator.swift#L792).

This expands this handling to consider any other network error, therefore providing more precise errors.

Since offline connection error detection was only in `HTTPClient`, I also moved that out and removed `NetworkError.offlineConnection`, so that we detect this when converting `NetworkError.networkError` instead.
@NachoSoto NachoSoto added the pr:fix A bug fix label Oct 25, 2023
@NachoSoto NachoSoto requested a review from a team October 25, 2023 17:25
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9dc1371) 85.97% compared to head (028f8c6) 85.98%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3346   +/-   ##
=======================================
  Coverage   85.97%   85.98%           
=======================================
  Files         237      237           
  Lines       17086    17086           
=======================================
+ Hits        14690    14691    +1     
+ Misses       2396     2395    -1     
Files Coverage Δ
Sources/Error Handling/ErrorUtils.swift 86.71% <100.00%> (+0.31%) ⬆️
Sources/Networking/HTTPClient/HTTPClient.swift 97.87% <100.00%> (-0.04%) ⬇️
Sources/Networking/HTTPClient/NetworkError.swift 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

Woah! This looks like a whole thing 😅 I don't see anything wrong with this though 🤷‍♂️

@NachoSoto NachoSoto merged commit ea952f0 into main Oct 26, 2023
21 checks passed
@NachoSoto NachoSoto deleted the sk-error-network-error branch October 26, 2023 03:25
NachoSoto added a commit that referenced this pull request Oct 26, 2023
**This is an automatic release.**

### New Features
* `PaywallColor`: change visibility of `Color.init(light:dark:)` to
`private` (#3345) via NachoSoto (@NachoSoto)
### RevenueCatUI
* `Paywalls`: new `.onPurchaseCompleted` overload with
`StoreTransaction` (#3323) via NachoSoto (@NachoSoto)
* `Paywalls`: finished template 5 (#3340) via NachoSoto (@NachoSoto)
* `Paywalls`: new `onDismiss` parameter for `presentPaywallIfNeeded`
(#3342) via NachoSoto (@NachoSoto)
* `Paywalls`: disable shimmering on footer loading view (#3324) via
NachoSoto (@NachoSoto)
### Bugfixes
* `ErrorUtils.purchasesError(withSKError:)`: handle `URLError`s (#3346)
via NachoSoto (@NachoSoto)
### Other Changes
* `Paywalls`: add identifier to events (#3332) via Josh Holtz
(@joshdholtz)
* `Paywalls`: create new event session when paywall appears (#3330) via
Josh Holtz (@joshdholtz)
* `HTTPClient`: verbose logs for request IDs (#3320) via NachoSoto
(@NachoSoto)
* `Paywalls Tester`: fix `macOS` build (#3341) via NachoSoto
(@NachoSoto)
* `ProductFetcherSK1`: enable `TimingUtil` log (#3327) via NachoSoto
(@NachoSoto)
* `Paywall Tester`: fixed paywall presentation (#3339) via NachoSoto
(@NachoSoto)
* `CI`: replace Carthage build jobs with `xcodebuild` (#3338) via
NachoSoto (@NachoSoto)
* `Integration Tests`: use repetition count from test plan (#3329) via
NachoSoto (@NachoSoto)
* `Integration Tests`: new logs for troubleshooting flaky tests (#3328)
via NachoSoto (@NachoSoto)
* `CircleCI`: change iOS 17 job to use M1 Large resource (#3322) via
NachoSoto (@NachoSoto)
* `Paywalls Tester`: fix release build (#3321) via NachoSoto
(@NachoSoto)
* `Paywalls`: enable all iOS 17 tests (#3331) via NachoSoto (@NachoSoto)
* `CI`: added workaround for Snapshots in `Xcode Cloud` (#2857) via
NachoSoto (@NachoSoto)
* `StoreKit 1`: disabled `finishTransactions` log on observer mode
(#3314) via NachoSoto (@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:fix A bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unknownError being returned when it should be a networkError
2 participants