-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
enhance: Improve error boundary #3010
Conversation
🦋 Changeset detectedLatest commit: 2da1446 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
fef9a3d
to
f051a13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmark
Benchmark suite | Current: 2da1446 | Previous: 3af5411 | Ratio |
---|---|---|---|
normalizeLong |
476 ops/sec (±0.49% ) |
479 ops/sec (±1.68% ) |
1.01 |
infer All |
9582 ops/sec (±1.51% ) |
9583 ops/sec (±1.35% ) |
1.00 |
denormalizeLong |
319 ops/sec (±2.18% ) |
318 ops/sec (±2.51% ) |
1.00 |
denormalizeLong donotcache |
856 ops/sec (±1.23% ) |
830 ops/sec (±1.10% ) |
0.97 |
denormalizeShort donotcache 500x |
1335 ops/sec (±0.16% ) |
1350 ops/sec (±0.26% ) |
1.01 |
denormalizeShort 500x |
952 ops/sec (±0.32% ) |
956 ops/sec (±0.23% ) |
1.00 |
denormalizeShort 500x withCache |
4892 ops/sec (±0.47% ) |
5022 ops/sec (±0.37% ) |
1.03 |
denormalizeLong with mixin Entity |
306 ops/sec (±0.46% ) |
299 ops/sec (±0.46% ) |
0.98 |
denormalizeLong withCache |
6766 ops/sec (±0.29% ) |
6468 ops/sec (±0.19% ) |
0.96 |
denormalizeLong All withCache |
6410 ops/sec (±0.61% ) |
6030 ops/sec (±0.14% ) |
0.94 |
denormalizeLong Query-sorted withCache |
6412 ops/sec (±0.26% ) |
6526 ops/sec (±0.23% ) |
1.02 |
denormalizeLongAndShort withEntityCacheOnly |
1554 ops/sec (±0.38% ) |
1536 ops/sec (±0.43% ) |
0.99 |
getResponse |
4622 ops/sec (±0.80% ) |
4881 ops/sec (±1.01% ) |
1.06 |
getResponse (null) |
5294287 ops/sec (±0.64% ) |
5444580 ops/sec (±0.35% ) |
1.03 |
getResponse (clear cache) |
295 ops/sec (±0.47% ) |
286 ops/sec (±0.52% ) |
0.97 |
getSmallResponse |
2471 ops/sec (±0.31% ) |
2539 ops/sec (±0.26% ) |
1.03 |
getSmallInferredResponse |
1852 ops/sec (±0.35% ) |
1832 ops/sec (±0.17% ) |
0.99 |
getResponse Query-sorted |
4493 ops/sec (±0.44% ) |
4557 ops/sec (±0.17% ) |
1.01 |
getResponse Collection |
4961 ops/sec (±1.09% ) |
4859 ops/sec (±0.65% ) |
0.98 |
get Collection |
4519 ops/sec (±0.43% ) |
4751 ops/sec (±0.41% ) |
1.05 |
setLong |
480 ops/sec (±0.37% ) |
481 ops/sec (±0.54% ) |
1.00 |
setLongWithMerge |
191 ops/sec (±0.27% ) |
189 ops/sec (±0.40% ) |
0.99 |
setLongWithSimpleMerge |
205 ops/sec (±0.28% ) |
205 ops/sec (±0.25% ) |
1 |
setSmallResponse 500x |
861 ops/sec (±0.50% ) |
852 ops/sec (±0.38% ) |
0.99 |
This comment was automatically generated by workflow using github-action-benchmark.
2dbe429
to
6fe07fb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3010 +/- ##
==========================================
+ Coverage 98.46% 98.49% +0.02%
==========================================
Files 119 122 +3
Lines 2154 2186 +32
Branches 435 439 +4
==========================================
+ Hits 2121 2153 +32
Misses 21 21
Partials 12 12 ☔ View full report in Codecov by Sentry. |
Motivation
rethrowing non-network errors is ruining the stack trace, making it seem like there's a bug in Data Client.
Solution
Make our ErrorBoundary fully capable and flexible - and catch everything. Rely on our custom NetworkError to correctly craft the message to include status text, etc.
Everything applies to AsyncBoundary as well since it renders ErrorBoundary.
Adding
Fixing
Internal change
Test coverage now runs without typechecks as both are very memory intensive and somewhat cpu intensive.