Skip to content

ErrorResponse: drastically improved error messages, no more "unknown error"s#2660

Merged
NachoSoto merged 1 commit into
mainfrom
error-response-status-code
Jun 16, 2023
Merged

ErrorResponse: drastically improved error messages, no more "unknown error"s#2660
NachoSoto merged 1 commit into
mainfrom
error-response-status-code

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Description:

We've gotten several reports with error messages that aren't very helpful, like #2390.

Turns out that whenever the API returned a non-successful response but without an error code, we always produced "unknown error".
This improves the behavior to default to showing the status code. One example that was very obvious is setting up the SDK with an invalid API key.

Examples:

OfferingsManager.Error:

  • Before:
[Purchases] - ERROR: 🍎‼️ Error fetching offerings - The operation couldn’t be completed. (RevenueCat.OfferingsManager.Error error 0.)
Underlying error: The operation couldn’t be completed. (RevenueCat.NetworkError error 6.)
  • After:
[Purchases] - ERROR: 🍎‼️ Error fetching offerings - The operation couldn’t be completed. (RevenueCat.OfferingsManager.Error error 0.)
Underlying error: Unknown error. API request failed with status code 401

BackendError:

  • Before:
[Purchases] - ERROR: 😿‼️ Unknown error.
  • After:
[Purchases] - ERROR: 😿‼️ Unknown error. API request failed with status code 401

Request debug logs:

  • Before:
[Purchases] - DEBUG: ℹ️ API request failed: GET /v1/subscribers/$RCAnonymousID:988f9b85609b4a57af255750bec1fbb2/offerings: Unknown error.
  • After:
[Purchases] - DEBUG: ℹ️ API request failed: GET /v1/subscribers/$RCAnonymousID:988f9b85609b4a57af255750bec1fbb2/offerings: Unknown error. API request failed with status code 401

CustomerInfoManager:

  • Before:
[Purchases] - WARN: ⚠️ Attempt to update CustomerInfo from network failed.
The operation couldn’t be completed. (RevenueCat.BackendError error 0.)
Underlying error: Unknown error
  • After:
[Purchases] - WARN: ⚠️ Attempt to update CustomerInfo from network failed.
The operation couldn’t be completed. (RevenueCat.BackendError error 0.)
Underlying error: Unknown error. API request failed with status code 401

@NachoSoto NachoSoto added the pr:fix A bug fix label Jun 15, 2023
@NachoSoto NachoSoto requested a review from a team June 15, 2023 18:43

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.

Also added this

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.

This was also a big part of this PR, we'll stop showing "The operation couldn’t be completed." whenever we log a NetworkError.

@NachoSoto NachoSoto force-pushed the error-response-status-code branch from 59c7d61 to cdb7d48 Compare June 15, 2023 18:54
@NachoSoto NachoSoto changed the base branch from main to status-code-unauthorized June 15, 2023 18:55
@NachoSoto NachoSoto force-pushed the error-response-status-code branch from cdb7d48 to d510559 Compare June 15, 2023 20:11
@NachoSoto NachoSoto force-pushed the status-code-unauthorized branch from 49d6550 to 00fdc7b Compare June 15, 2023 20:18
@NachoSoto NachoSoto force-pushed the error-response-status-code branch 2 times, most recently from fb8ab5c to 0e289b1 Compare June 15, 2023 20:44
@NachoSoto NachoSoto force-pushed the status-code-unauthorized branch from 00fdc7b to 05e0d79 Compare June 15, 2023 22:16
@NachoSoto NachoSoto force-pushed the error-response-status-code branch from 0e289b1 to 0cfc9ef Compare June 15, 2023 22:16
@NachoSoto NachoSoto force-pushed the status-code-unauthorized branch from 05e0d79 to 0812041 Compare June 15, 2023 23:24
@NachoSoto NachoSoto force-pushed the error-response-status-code branch 4 times, most recently from 20d23d2 to 27dd6aa Compare June 15, 2023 23:39
@codecov

codecov Bot commented Jun 15, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2660 (0812041) into main (20dcc86) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

❗ Current head 0812041 differs from pull request most recent head 73020c7. Consider uploading reports for the commit 73020c7 to get more accurate results

@@            Coverage Diff             @@
##             main    #2660      +/-   ##
==========================================
- Coverage   86.43%   86.38%   -0.06%     
==========================================
  Files         208      207       -1     
  Lines       14746    14656      -90     
==========================================
- Hits        12746    12660      -86     
+ Misses       2000     1996       -4     
Impacted Files Coverage Δ
Sources/Networking/HTTPClient/HTTPStatusCode.swift 100.00% <100.00%> (ø)

... and 16 files with indirect coverage changes

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

Nice!

Base automatically changed from status-code-unauthorized to main June 16, 2023 14:56
…n error"s

We've gotten several reports with error messages that aren't very helpful, like #2390.

Turns out that whenever the API returned a non-successful response but without an error code, we always produced "unknown error".
This improves the behavior to default to showing the status code. One example that was very obvious is setting up the SDK with an invalid API key.

All these errors are improved now:

`OfferingsManager.Error`:

- Before:
```
[Purchases] - ERROR: 🍎‼️ Error fetching offerings - The operation couldn’t be completed. (RevenueCat.OfferingsManager.Error error 0.)
Underlying error: The operation couldn’t be completed. (RevenueCat.NetworkError error 6.)
```

- After:
```
[Purchases] - ERROR: 🍎‼️ Error fetching offerings - The operation couldn’t be completed. (RevenueCat.OfferingsManager.Error error 0.)
Underlying error: Unknown error. API request failed with status code 401
```

`BackendError`:
- Before:
```
[Purchases] - ERROR: 😿‼️ Unknown error.
```

- After:
```
[Purchases] - ERROR: 😿‼️ Unknown error. API request failed with status code 401
```

Request debug logs:
- Before:
```
[Purchases] - DEBUG: ℹ️ API request failed: GET /v1/subscribers/$RCAnonymousID:988f9b85609b4a57af255750bec1fbb2/offerings: Unknown error.
```
- After:
```
[Purchases] - DEBUG: ℹ️ API request failed: GET /v1/subscribers/$RCAnonymousID:988f9b85609b4a57af255750bec1fbb2/offerings: Unknown error. API request failed with status code 401
```

`CustomerInfoManager`:
- Before:
```
[Purchases] - WARN: ⚠️ Attempt to update CustomerInfo from network failed.
The operation couldn’t be completed. (RevenueCat.BackendError error 0.)
Underlying error: Unknown error
```

- After:
```
[Purchases] - WARN: ⚠️ Attempt to update CustomerInfo from network failed.
The operation couldn’t be completed. (RevenueCat.BackendError error 0.)
Underlying error: Unknown error. API request failed with status code 401
```
@NachoSoto NachoSoto force-pushed the error-response-status-code branch from 27dd6aa to 73020c7 Compare June 16, 2023 15:47
@NachoSoto NachoSoto enabled auto-merge (squash) June 16, 2023 15:47
@NachoSoto NachoSoto merged commit 6d1cf97 into main Jun 16, 2023
@NachoSoto NachoSoto deleted the error-response-status-code branch June 16, 2023 16:19
NachoSoto pushed a commit that referenced this pull request Jun 22, 2023
**This is an automatic release.**

### Bugfixes
* `PurchasesOrchestrator`: update `CustomerInfoManager` cache after
processing transactions (#2676) via NachoSoto (@NachoSoto)
* `ErrorResponse`: drastically improved error messages, no more "unknown
error"s (#2660) via NachoSoto (@NachoSoto)
* `PaywallExtensions`: post purchases with `Offering` identifier (#2645)
via NachoSoto (@NachoSoto)
* Support `product_plan_identifier` for purchased subscriptions from
`Google Play` (#2654) via Josh Holtz (@joshdholtz)
### Performance Improvements
* `copy(with: VerificationResult)`: optimization to avoid copies (#2639)
via NachoSoto (@NachoSoto)
### Other Changes
* `ETagManager`: refactored e-tag creation and tests (#2671) via
NachoSoto (@NachoSoto)
* `getPromotionalOffer`: return `ErrorCode.ineligibleError` if receipt
is not found (#2678) via NachoSoto (@NachoSoto)
* `TimingUtil`: removed slow purchase logs (#2677) via NachoSoto
(@NachoSoto)
* `CI`: changed `Codecov` to `informational` (#2670) via NachoSoto
(@NachoSoto)
* `LoadShedderIntegrationTests`: verify requests are actually handled by
load shedder (#2663) via NachoSoto (@NachoSoto)
* `ETagManager.httpResultFromCacheOrBackend`: return response headers
(#2666) via NachoSoto (@NachoSoto)
* `Integration Tests`: added tests to verify 304 behavior (#2659) via
NachoSoto (@NachoSoto)
* `HTTPClient`: disable `URLSession` cache (#2668) via NachoSoto
(@NachoSoto)
* Documented `HTTPStatusCode.isSuccessfullySynced` (#2661) via NachoSoto
(@NachoSoto)
* `NetworkError.signatureVerificationFailed`: added status code to error
`userInfo` (#2657) via NachoSoto (@NachoSoto)
* `HTTPClient`: improved log for failed requests (#2669) via NachoSoto
(@NachoSoto)
* `ETagManager`: added new verbose logs (#2656) via NachoSoto
(@NachoSoto)
* `Signature Verification`: added test-only log for debugging invalid
signatures (#2658) via NachoSoto (@NachoSoto)
* Fixed `HTTPResponse.description` (#2664) via NachoSoto (@NachoSoto)
* Changed `Logger` to use `os_log` (#2608) via NachoSoto (@NachoSoto)
* `MainThreadMonitor`: increased threshold (#2662) via NachoSoto
(@NachoSoto)
* `debugRevenueCatOverlay`: display `receiptURL` (#2652) via NachoSoto
(@NachoSoto)
* `PurchaseTester`: added ability to display `debugRevenueCatOverlay`
(#2653) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: ability to close on `macOS`/`Catalyst`
(#2649) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: added support for `macOS` (#2648) via
NachoSoto (@NachoSoto)
* `LoadShedderIntegrationTests`: enable signature verification (#2655)
via NachoSoto (@NachoSoto)
* `ImageSnapshot`: fixed Xcode 15 compilation (#2651) via NachoSoto
(@NachoSoto)
* `OfferingsManager`: don't clear offerings cache timestamp when request
fails (#2359) via NachoSoto (@NachoSoto)
* `StoreKitObserverModeIntegrationTests`: added test for posting
renewals (#2590) via NachoSoto (@NachoSoto)
* Always initialize `StoreKit2TransactionListener` even on SK1 mode
(#2612) via NachoSoto (@NachoSoto)
* `ErrorUtils.missingReceiptFileError`: added receipt URL `userInfo`
context (#2650) via NachoSoto (@NachoSoto)
* Added `.xcprivacy` for Xcode 15 (#2619) via NachoSoto (@NachoSoto)
* `Trusted Entitlements`: added debug log with
`ResponseVerificationMode` (#2647) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: simplified title (#2641) via NachoSoto
(@NachoSoto)
* Simplified `Purchases.updateAllCachesIfNeeded` (#2626) via NachoSoto
(@NachoSoto)
* `HTTPResponseTests`: fixed disabled test (#2643) via NachoSoto
(@NachoSoto)
* Add `InternalDangerousSettings.forceSignatureFailures` (#2635) via
NachoSoto (@NachoSoto)
* `IntegrationTests`: explicit `StoreKit 1` mode (#2636) via NachoSoto
(@NachoSoto)
* `Signing`: removed API for loading key from a file (#2638) via
NachoSoto (@NachoSoto)
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.

2 participants