This is an automated email from the ASF dual-hosted git repository. enzomartellucci pushed a commit to branch enxdev/fix/dashboard-charts-infinite-loading in repository https://gitbox.apache.org/repos/asf/superset.git
commit 4d64caa072da2ba1d9758c03e6ac9c04f683380a Author: Enzo Martellucci <[email protected]> AuthorDate: Thu Feb 12 18:03:22 2026 +0100 fix(charts): improve error display for failed charts in dashboards --- .../src/components/Chart/Chart.test.tsx | 90 ++++++++++++++++++++++ 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, 130 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..63fd3ac7642 --- /dev/null +++ b/superset-frontend/src/components/Chart/Chart.test.tsx @@ -0,0 +1,90 @@ +/** + * 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..3b718b6335e 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) 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.lower() ) ) 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.
