Repository navigation
fix(dashboard): make chart error messages expandable again - #42491
Conversation
Chart render errors inside a dashboard showed as a flat, unexpandable "Data error" card, while the same error in Explore still offered a "See more" toggle with the full stack trace. Regression from apache#31858, which removed the source-based expandable error rendering from ErrorAlert. - Forward chartStackTrace from the dashboard Chart wrapper into ChartContainer, mirroring ExploreChartPanel, so the stack trace is reachable behind "See more". - In ErrorMessageWithStackTrace, when no stack trace, link, or explicit details are available, surface the error subtitle in the collapsible section instead of inline, so the alert always offers an expand affordance rather than a bare "Data error". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bito Review Skipped - Source Branch Not Found |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The flagged issue is correct. The current implementation uses a value comparison ( To resolve this, you should track whether the fallback logic was triggered rather than comparing the values. You can introduce a boolean flag or check if Proposed FixUpdate // Track if descriptionDetails was explicitly provided
const hasExplicitDetails = !!descriptionDetails;
const message = hasExplicitDetails ? undefined : subtitle;This ensures that if Regarding other comments: I have checked the PR and there are no other review comments in the provided context. If you have additional concerns or comments you would like me to review, please provide them. superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx |
There was a problem hiding this comment.
Pull request overview
Restores parity between Explore and Dashboard chart error rendering by ensuring dashboard chart errors can expand (“See more”) to reveal details/stack traces again, and by guaranteeing an expand affordance even when no stack trace/link is available.
Changes:
- Forward
chartStackTracefrom the dashboard chart wrapper intoChartContainerso dashboard failures can render expandable details. - Adjust
ErrorMessageWithStackTraceso a subtitle-only error uses the collapsible details section (enabling “See more”) rather than rendering as a flat, truncated message. - Add regression tests covering both behaviors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx | Forwards chartStackTrace into ChartContainer for dashboard charts. |
| superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx | Adds regression test asserting chartStackTrace is passed through. |
| superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx | Ensures subtitle-only errors still get collapsible details (“See more”). |
| superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx | Adds regression test asserting “See more” exists even without a stack trace. |
Drop the shared ErrorMessageWithStackTrace change after review feedback; the dashboard chart error regression is fully addressed by forwarding chartStackTrace into ChartContainer, matching Explore's behavior.
|
Thanks for the review. I've scoped this PR down to the core regression fix: the one-line I reverted the shared |
|
@yousoph can you add before and after images to this PR? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42491 +/- ##
==========================================
+ Coverage 64.50% 64.52% +0.02%
==========================================
Files 2803 2803
Lines 158383 158455 +72
Branches 36152 36169 +17
==========================================
+ Hits 102164 102243 +79
+ Misses 54216 54209 -7
Partials 2003 2003
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SUMMARY
When a chart fails to render inside a dashboard, the error is shown as a flat, truncated alert with no way to expand it for more detail. The same chart error in Explore still shows an expandable "See more" with the full stack trace.
This is a regression from #31858 ("chore: refactor Alert-related components"), which reworked
ErrorAlertso the "See more" toggle only renders whendescriptionDetailsis truthy.descriptionDetailsis derived fromlink || stackTrace. Explore'sExploreChartPanelforwardschartStackTrace, so its toggle renders; the dashboard chart wrapper (gridComponents/Chart/Chart.tsx) never forwardedchartStackTrace, sodescriptionDetailswasundefinedand no expand affordance appeared.Changes
dashboard/components/gridComponents/Chart/Chart.tsx— forwardchartStackTraceinto<ChartContainer>, mirroringExploreChartPanel. Restores "See more" / stack-trace parity for the common case (chart errors that carry a stack trace).components/ErrorMessage/ErrorMessageWithStackTrace.tsx— when the only detail available is the subtitle (no stack trace, link, or explicitdescriptionDetails), surface it inside the collapsible "See more" section instead of inline. This guarantees an expand affordance rather than a flat, unexpandable card even when there is no stack trace. Note this path is shared by allErrorMessageWithStackTraceconsumers, not just dashboard charts.TESTING INSTRUCTIONS
Chart.test.tsx: the dashboard chart wrapper forwardschartStackTracetoChartContainer.ChartErrorMessage.test.tsx: an error with no stack trace still exposes a "See more" toggle.Before:

After:

ADDITIONAL INFORMATION