chore(native-filters): Ensure consistent error handling #24206
Conversation
john-bodley
force-pushed
the
john-bodley--native-filter-errors
branch
3 times, most recently
from
f047200
to
96d717a
Compare
john-bodley
force-pushed
the
john-bodley--native-filter-errors
branch
from
96d717a
to
319a4cc
Compare
john-bodley
force-pushed
the
john-bodley--native-filter-errors
branch
from
319a4cc
to
fc3b0fe
Compare
Codecov Report
@@ Coverage Diff @@
## master #24206 +/- ##
==========================================
- Coverage 68.30% 68.19% -0.11%
==========================================
Files 1952 1957 +5
Lines 75432 75592 +160
Branches 8191 8221 +30
==========================================
+ Hits 51521 51553 +32
- Misses 21807 21932 +125
- Partials 2104 2107 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Thanks for the commit @michael-s-molina. I left a couple of comments.
| }); | ||
| setError( | ||
| error.message || error.error || t('Check configuration'), |
There was a problem hiding this comment.
@michael-s-molina do we not need to preserve this messaging?
There was a problem hiding this comment.
Looking at getClientErrorObject implementation, an error is always returned and it deals with the generic case as well. In my opinion it offers a more standard treatment to the exceptions.
| getClientErrorObject(error).then(parsedError => { | ||
| errorRef.current = parsedError.errors?.[0]; | ||
| }); | ||
| error.json().then(body => { |
There was a problem hiding this comment.
Similar to the previous question.
Co-authored-by: Michael S. Molina <michael.s.molina@gmail.com> (cherry picked from commit 674da1b)
mistercrunch
added
🏷️ bot
SUMMARY
This PR ensures that the native filter components adhere to the underlying
SupersetErrorreturned by the server and render them accordingly via theErrorMessageWithStackTracecomponent (which now accepts a fallback component) which provides custom overrides.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI, added unit tests, and manual testing. Below are examples of the
ErrorMessageWithStackTracecomponent using an Airbnb internal custom override when users don't have access to the underlying dataset.Before
After
ADDITIONAL INFORMATION