Skip to content

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

Merged
NachoSoto merged 1 commit into
mainfrom
sk-error-network-error
Oct 26, 2023
Merged

ErrorUtils.purchasesError(withSKError:): handle URLErrors#3346
NachoSoto merged 1 commit into
mainfrom
sk-error-network-error

Conversation

@NachoSoto

Copy link
Copy Markdown
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

codecov Bot commented Oct 25, 2023

Copy link
Copy Markdown

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.

@joshdholtz joshdholtz left a comment

Copy link
Copy Markdown
Member

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
@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