-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
fix: error alerts js crash #17015
fix: error alerts js crash #17015
Conversation
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.
Thanks for the additional checks!
Codecov Report
@@ Coverage Diff @@
## master #17015 +/- ##
=======================================
Coverage 76.93% 76.94%
=======================================
Files 1030 1030
Lines 55085 55088 +3
Branches 7477 7480 +3
=======================================
+ Hits 42382 42385 +3
Misses 12452 12452
Partials 251 251
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@etr2460 yeah... |
SUMMARY
When template params are malformed in sql lab, we return an error from the backend that doesn't include an error level. This caused the frontend to crash
I think this was likely caused by #16917 or #16852, but i don't have enough context on the backend right now to resolve it there. I figure we should also harden the frontend to handle this case, so i added the check there. @ofekisr, could you follow up your PR and ensure that all errors you pass from the backend contain an ErrorLevel? We have helper functions to construct those errors, so it shouldn't be too difficult
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI, write a query with a malformed jinja template param and see the error properly
ADDITIONAL INFORMATION
to: @rusackas @pkdotson @graceguo-supercat @michael-s-molina