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

IgnoreDecodeErrors: log decoding error #2778

Merged
merged 4 commits into from
Jul 12, 2023
Merged

Conversation

NachoSoto
Copy link
Contributor

This will be used for decoding the upcoming paywall data.
If we end up with values that we can't parse (like an unknown template), we need to be able to see that in the logs instead of just decoding no paywalls.

I made this debug since we expected it and recovered from it.

This also improves the error.

Before:

DEBUG: ℹ️ Couldn't decode data from json.
Error: The data couldn’t be read because it isn’t in the correct format.

After:

[codable] DEBUG: ℹ️ Couldn't decode 'E' from json.
Error: Swift.DecodingError.dataCorrupted(Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "e", intValue: nil)], debugDescription: "Cannot initialize E from invalid String value e3", underlyingError: nil))

@NachoSoto NachoSoto requested a review from a team July 10, 2023 22:40
@NachoSoto NachoSoto force-pushed the ignore-decode-errors-log branch from adc3d19 to 7d32e8b Compare July 10, 2023 22:42

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
We get a lot of flaky failures in CI due to `TestLogHandler` exceeding its duration. I haven't been able reproduce that locally, but it's likely due to the fact that it relies on the autorelease pool releasing the instance, which in turn removes the `LogMessageObserver`.
To fix that, this PR avoids creating custom `TestLogHandler` that rely on the lifetime of the test, and instead explicitly set it to `nil` in `tearDown`.

I implemented that in `TestCase` (now also available for backend tests) so all our test classes can take advantage of that.
@NachoSoto
Copy link
Contributor Author

This has exposed the TestLogHandler issue even more. Hopefully #2784 will fix it.

@NachoSoto NachoSoto force-pushed the ignore-decode-errors-log branch from 7d32e8b to 85c9fb2 Compare July 11, 2023 16:59
@NachoSoto NachoSoto changed the base branch from main to test-log-handler-lifetime July 11, 2023 16:59
@NachoSoto NachoSoto force-pushed the ignore-decode-errors-log branch from 85c9fb2 to 4b16415 Compare July 11, 2023 17:18
@NachoSoto NachoSoto force-pushed the ignore-decode-errors-log branch from 4b16415 to c700033 Compare July 11, 2023 17:55
This will be used for decoding the upcoming paywall data.
If we end up with values that we can't parse (like an unknown template), we need to be able to see that in the logs instead of just decoding no paywalls.

I made this `debug` since we expected it and recovered from it.

This also improves the error.
Before:
```
DEBUG: ℹ️ Couldn't decode data from json.
Error: The data couldn’t be read because it isn’t in the correct format.
```

After:
```
[codable] DEBUG: ℹ️ Couldn't decode 'E' from json.
Error: Swift.DecodingError.dataCorrupted(Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "e", intValue: nil)], debugDescription: "Cannot initialize E from invalid String value e3", underlyingError: nil))
```
@NachoSoto NachoSoto force-pushed the ignore-decode-errors-log branch from c700033 to e7d4877 Compare July 11, 2023 17:56
@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

Merging #2778 (e5846d8) into test-log-handler-lifetime (e5846d8) will not change coverage.
The diff coverage is n/a.

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

@@                    Coverage Diff                     @@
##           test-log-handler-lifetime    #2778   +/-   ##
==========================================================
  Coverage                      86.48%   86.48%           
==========================================================
  Files                            214      214           
  Lines                          15379    15379           
==========================================================
  Hits                           13300    13300           
  Misses                          2079     2079           

Base automatically changed from test-log-handler-lifetime to main July 12, 2023 13:34
@NachoSoto NachoSoto merged commit c5552fa into main Jul 12, 2023
@NachoSoto NachoSoto deleted the ignore-decode-errors-log branch July 12, 2023 13:35
NachoSoto added a commit that referenced this pull request Jul 12, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using `IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases

- Made `DefaultDecodable` and `AnyDecodable` `Sendable`
NachoSoto added a commit that referenced this pull request Jul 13, 2023
### Changes:
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 13, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 13, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 13, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 20, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 23, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 24, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 24, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 25, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 26, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 27, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Jul 31, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 3, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 7, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 9, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 11, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 14, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 17, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 24, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 28, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Aug 31, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 1, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 6, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 6, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 6, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 7, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 8, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 14, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 14, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
NachoSoto added a commit that referenced this pull request Sep 15, 2023
- Added `Offering.paywall`
- Decoding `PaywallData` in `OfferingsResponse`, using
`IgnoreDecodeErrors` (this will be better after #2778)
- New `PaywallData` `struct`
- Added new APIs to testers
- Testing paywall deserialization from `Offerings`
- Testing paywall deserialization separately to check edge cases
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.

None yet

2 participants