This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new b565128fe76 fix(charts): improve error display for failed charts in
dashboards (#37939)
b565128fe76 is described below
commit b565128fe76d4167431d5eed263cd8236d86cb0b
Author: Enzo Martellucci <[email protected]>
AuthorDate: Sat Feb 21 00:14:48 2026 +0100
fix(charts): improve error display for failed charts in dashboards (#37939)
---
.../src/components/Chart/Chart.test.tsx | 88 ++++++++++++++++++++++
superset-frontend/src/components/Chart/Chart.tsx | 16 +++-
superset/db_engine_specs/gsheets.py | 7 +-
tests/unit_tests/db_engine_specs/test_gsheets.py | 20 +++++
4 files changed, 128 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/src/components/Chart/Chart.test.tsx
b/superset-frontend/src/components/Chart/Chart.test.tsx
new file mode 100644
index 00000000000..53028d2d1e9
--- /dev/null
+++ b/superset-frontend/src/components/Chart/Chart.test.tsx
@@ -0,0 +1,88 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, screen } from 'spec/helpers/testing-library';
+import '@testing-library/jest-dom';
+import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
+import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
+import Chart from './Chart';
+import type { Actions } from './Chart';
+
+const mockActions: Actions = {
+ logEvent: jest.fn() as unknown as Actions['logEvent'],
+ chartRenderingFailed: jest.fn() as unknown as
Actions['chartRenderingFailed'],
+ chartRenderingSucceeded:
+ jest.fn() as unknown as Actions['chartRenderingSucceeded'],
+ postChartFormData: jest.fn() as unknown as Actions['postChartFormData'],
+};
+
+const baseProps = {
+ chartId: 1,
+ width: 800,
+ height: 600,
+ actions: mockActions,
+ formData: { datasource: '1__table', viz_type: 'table' },
+ vizType: 'table',
+ setControlValue: jest.fn(),
+};
+
+test('shows backend error instead of loading spinner when datasource is still
a placeholder', () => {
+ render(
+ <Chart
+ {...baseProps}
+ chartStatus="failed"
+ chartAlert="Your default credentials were not found."
+ datasource={PLACEHOLDER_DATASOURCE}
+ datasetsStatus={ResourceStatus.Loading}
+ queriesResponse={[
+ {
+ errors: [
+ {
+ error_type: 'GENERIC_BACKEND_ERROR',
+ message: 'Your default credentials were not found.',
+ extra: {
+ issue_codes: [{ code: 1011, message: 'Issue 1011' }],
+ },
+ level: 'error',
+ },
+ ],
+ },
+ ]}
+ />,
+ );
+
+ expect(
+ screen.getByText(/Your default credentials were not found/),
+ ).toBeInTheDocument();
+});
+
+test('shows loading spinner for client-side errors without errors array when
datasource is still a placeholder', () => {
+ render(
+ <Chart
+ {...baseProps}
+ chartStatus="failed"
+ chartAlert="Some client-side error"
+ datasource={PLACEHOLDER_DATASOURCE}
+ datasetsStatus={ResourceStatus.Loading}
+ queriesResponse={[{}]}
+ />,
+ );
+
+ expect(screen.getByRole('status')).toBeInTheDocument();
+ expect(screen.queryByText(/Some client-side error/)).not.toBeInTheDocument();
+});
diff --git a/superset-frontend/src/components/Chart/Chart.tsx
b/superset-frontend/src/components/Chart/Chart.tsx
index 9a05bb270fb..7ab23a2260d 100644
--- a/superset-frontend/src/components/Chart/Chart.tsx
+++ b/superset-frontend/src/components/Chart/Chart.tsx
@@ -168,6 +168,11 @@ const LoadingDiv = styled.div`
transform: translate(-50%, -50%);
`;
+const ErrorContainer = styled.div<{ height: number }>`
+ height: ${p => p.height}px;
+ overflow: auto;
+`;
+
const MessageSpan = styled.span`
display: block;
text-align: center;
@@ -261,7 +266,10 @@ class Chart extends PureComponent<ChartProps, {}> {
const message = chartAlert || queryResponse?.message;
// if datasource is still loading, don't render JS errors
+ // but always show backend API errors (which have an errors array)
+ // so users can see real issues like auth failures
if (
+ !error &&
chartAlert !== undefined &&
chartAlert !== NONEXISTENT_DATASET &&
datasource === PLACEHOLDER_DATASOURCE &&
@@ -353,8 +361,12 @@ class Chart extends PureComponent<ChartProps, {}> {
const isLoading = chartStatus === 'loading';
if (chartStatus === 'failed') {
- return queriesResponse?.map(item =>
- this.renderErrorMessage(item as ChartErrorType),
+ return (
+ <ErrorContainer height={height}>
+ {queriesResponse?.map(item =>
+ this.renderErrorMessage(item as ChartErrorType),
+ )}
+ </ErrorContainer>
);
}
diff --git a/superset/db_engine_specs/gsheets.py
b/superset/db_engine_specs/gsheets.py
index 780f92cc750..9692db9d9a6 100644
--- a/superset/db_engine_specs/gsheets.py
+++ b/superset/db_engine_specs/gsheets.py
@@ -203,13 +203,18 @@ class GSheetsEngineSpec(ShillelaghEngineSpec):
In case the token was manually revoked on Google side, `google-auth`
will
try to automatically refresh credentials, but it fails since it only
has the
access token. This override catches this scenario as well.
+
+ Also catches the case where no credentials are configured at all
+ (missing Application Default Credentials).
"""
+ error_message = str(ex).lower()
return (
g
and hasattr(g, "user")
and (
isinstance(ex, cls.oauth2_exception)
- or "credentials do not contain the necessary fields" in str(ex)
+ or "credentials do not contain the necessary fields" in
error_message
+ or "default credentials were not found" in error_message
)
)
diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py
b/tests/unit_tests/db_engine_specs/test_gsheets.py
index a2406c18fc3..aac67ea1fe2 100644
--- a/tests/unit_tests/db_engine_specs/test_gsheets.py
+++ b/tests/unit_tests/db_engine_specs/test_gsheets.py
@@ -757,6 +757,26 @@ def test_needs_oauth2_with_credentials_error(mocker:
MockerFixture) -> None:
assert GSheetsEngineSpec.needs_oauth2(ex) is True
+def test_needs_oauth2_with_default_credentials_not_found(
+ mocker: MockerFixture,
+) -> None:
+ """
+ Test that needs_oauth2 returns True when Application Default Credentials
+ are not configured.
+ """
+ from superset.db_engine_specs.gsheets import GSheetsEngineSpec
+
+ g = mocker.patch("superset.db_engine_specs.gsheets.g")
+ g.user = mocker.MagicMock()
+
+ ex = Exception(
+ "Your default credentials were not found. To set up Application
Default "
+ "Credentials, see
https://cloud.google.com/docs/authentication/external/";
+ "set-up-adc for more information."
+ )
+ assert GSheetsEngineSpec.needs_oauth2(ex) is True
+
+
def test_needs_oauth2_with_other_error(mocker: MockerFixture) -> None:
"""
Test that needs_oauth2 returns False for other errors.
