From 74a6c1c6ce7cf8bd82fbca050676bf5b1616e40f Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 10 Jan 2025 17:56:41 +0300 Subject: [PATCH 01/18] test(Error): stories --- .../ErrorMessage/ErrorMessage.stories.tsx | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx new file mode 100644 index 0000000000000..59fe7bfe1d58d --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx @@ -0,0 +1,71 @@ +/** + * 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 BasicErrorAlert from './BasicErrorAlert'; +import ErrorAlert from './ErrorAlert'; + +export default { + title: 'ErrorMessage', +}; + +export const InteractiveBasicErrorAlert = (args: any) => ( + +); + +InteractiveBasicErrorAlert.argTypes = { + level: { + control: { + type: 'select', + }, + options: ['error', 'warning', 'info'], + }, +}; + +InteractiveBasicErrorAlert.args = { + title: 'Title', + body: 'Body', + level: 'error', +}; + +export const InteractiveErrorAlert = (args: any) => ; + +InteractiveErrorAlert.argTypes = { + level: { + control: { + type: 'select', + }, + options: ['error', 'warning', 'info'], + }, + source: { + control: { + type: 'select', + }, + options: ['dashboard', 'explore', 'sqllab', 'crud'], + }, +}; + +InteractiveErrorAlert.args = { + title: 'Title', + copyText: 'Copy error message', + level: 'error', + source: 'dashboard', + subtitle: 'Subtitle', + body: 'Body', + description: 'Description', +}; From a9f61e50d3c26803c00f483ea6f1f64c40fead1a Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 12:04:50 +0300 Subject: [PATCH 02/18] chore(Error Messages): update filter bar errors --- .../ErrorMessage/BasicErrorAlert.tsx | 24 ++++++++++++++++++- .../ErrorMessage/ErrorMessage.stories.tsx | 1 + .../FilterBar/FilterControls/FilterValue.tsx | 2 ++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx index 39ae748970072..1dcb5393c231a 100644 --- a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx +++ b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx @@ -18,17 +18,20 @@ */ import { ErrorLevel, styled, useTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; +import { Tooltip } from '../Tooltip'; const StyledContainer = styled.div<{ level: ErrorLevel }>` display: flex; flex-direction: row; + align-items: center; background-color: ${({ level, theme }) => theme.colors[level].light2}; border-radius: ${({ theme }) => theme.borderRadius}px; border: 1px solid ${({ level, theme }) => theme.colors[level].base}; color: ${({ level, theme }) => theme.colors[level].dark2}; - padding: ${({ theme }) => theme.gridUnit * 2}px; + padding: ${({ theme }) => theme.gridUnit * 1}px; margin-bottom: ${({ theme }) => theme.gridUnit}px; width: 100%; + min-width: max-content; `; const StyledContent = styled.div` @@ -46,16 +49,35 @@ interface BasicErrorAlertProps { title: string; body: string; level?: ErrorLevel; + bodyOnTooltip?: boolean; } export default function BasicErrorAlert({ body, level = 'error', title, + bodyOnTooltip, }: BasicErrorAlertProps) { const theme = useTheme(); const iconColor = theme.colors[level].base; + if (bodyOnTooltip) { + return ( + + + {level === 'error' ? ( + + ) : ( + + )} + + {title} + + + + ); + } + return ( {level === 'error' ? ( diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx index 59fe7bfe1d58d..6519a7ba546b2 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx @@ -41,6 +41,7 @@ InteractiveBasicErrorAlert.args = { title: 'Title', body: 'Body', level: 'error', + bodyOnTooltip: false, }; export const InteractiveErrorAlert = (args: any) => ; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 454d9060321e9..29059b819902c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -106,6 +106,7 @@ const FilterValue: FC = ({ const dashboardId = useSelector( state => state.dashboardInfo.id, ); + const [error, setError] = useState(); const [formData, setFormData] = useState>({ inView: false, @@ -310,6 +311,7 @@ const FilterValue: FC = ({ title={t('Cannot load filter')} body={error.error} level="error" + bodyOnTooltip /> } /> From 1fb351ee1d2bf382cf4ed0a7c2926f84925a657a Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 20:18:01 +0300 Subject: [PATCH 03/18] feat(NetworkError): add error message component --- .../src/query/getClientErrorObject.ts | 19 +++++++- .../src/components/Chart/Chart.tsx | 4 +- .../ErrorMessage/NetworkErrorMessage.tsx | 43 +++++++++++++++++++ .../src/setup/setupErrorMessages.ts | 5 +++ 4 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx diff --git a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts index dd26aec6192d7..e24be92338e04 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts @@ -177,11 +177,25 @@ export function getClientErrorObject( } if ( - response instanceof TypeError && - response.message === 'Failed to fetch' + (response instanceof TypeError && + response.message === 'Failed to fetch') || + (response instanceof TypeError && + response.message.includes('NetworkError')) ) { resolve({ error: t('Network error'), + message: t( + 'Network Error when attempting to load resource. Please check your internet connection and try again.', + ), + errors: [ + { + error_type: ErrorTypeEnum.FRONTEND_NETWORK_ERROR, + level: 'error', + message: + 'Network Error when attempting to load resource. Please check your internet connection and try again.', + extra: {}, + }, + ], }); return; } @@ -254,6 +268,7 @@ export function getClientErrorObject( } // fall back to Response.statusText or generic error of we cannot read the response + console.log({ response }); let error = (response as any).statusText || (response as any).message; if (!error) { // eslint-disable-next-line no-console diff --git a/superset-frontend/src/components/Chart/Chart.tsx b/superset-frontend/src/components/Chart/Chart.tsx index 3526a26b3f9ce..1dec8da5737aa 100644 --- a/superset-frontend/src/components/Chart/Chart.tsx +++ b/superset-frontend/src/components/Chart/Chart.tsx @@ -273,8 +273,8 @@ class Chart extends PureComponent { key={chartId} chartId={chartId} error={error} - subtitle={{message}} - copyText={message} + subtitle={{error?.message || message}} + copyText={error?.message || message} link={queryResponse ? queryResponse.link : undefined} source={dashboardId ? ChartSource.Dashboard : ChartSource.Explore} stackTrace={chartStackTrace} diff --git a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx new file mode 100644 index 0000000000000..ceb7c5ce194e1 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx @@ -0,0 +1,43 @@ +/** + * 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 { t } from '@superset-ui/core'; + +import { ErrorMessageComponentProps } from './types'; +import ErrorAlert from './ErrorAlert'; + +function NetworkErrorMessage({ + error, + source = 'dashboard', + subtitle, +}: ErrorMessageComponentProps) { + const { level, message } = error; + + return ( + + ); +} + +export default NetworkErrorMessage; diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts index e4c2380150c9d..f7d10806b65ab 100644 --- a/superset-frontend/src/setup/setupErrorMessages.ts +++ b/superset-frontend/src/setup/setupErrorMessages.ts @@ -25,6 +25,7 @@ import ParameterErrorMessage from 'src/components/ErrorMessage/ParameterErrorMes import DatasetNotFoundErrorMessage from 'src/components/ErrorMessage/DatasetNotFoundErrorMessage'; import InvalidSQLErrorMessage from 'src/components/ErrorMessage/InvalidSQLErrorMessage'; import OAuth2RedirectMessage from 'src/components/ErrorMessage/OAuth2RedirectMessage'; +import NetworkErrorMessage from 'src/components/ErrorMessage/NetworkErrorMessage'; import setupErrorMessagesExtra from './setupErrorMessagesExtra'; @@ -163,5 +164,9 @@ export default function setupErrorMessages() { ErrorTypeEnum.RESULT_TOO_LARGE_ERROR, DatabaseErrorMessage, ); + errorMessageComponentRegistry.registerValue( + ErrorTypeEnum.FRONTEND_NETWORK_ERROR, + NetworkErrorMessage, + ); setupErrorMessagesExtra(); } From adf48047f674773779c27ed6c034cbb5fdc13b7f Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 20:20:08 +0300 Subject: [PATCH 04/18] chore(log): remove console log --- .../packages/superset-ui-core/src/query/getClientErrorObject.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts index e24be92338e04..d194051fbe9c5 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts @@ -268,7 +268,6 @@ export function getClientErrorObject( } // fall back to Response.statusText or generic error of we cannot read the response - console.log({ response }); let error = (response as any).statusText || (response as any).message; if (!error) { // eslint-disable-next-line no-console From 8d21492697bff5a724c6704b16b8e83e6b29341b Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 20:25:53 +0300 Subject: [PATCH 05/18] chore(error): localize error --- .../superset-ui-core/src/query/getClientErrorObject.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts index d194051fbe9c5..8e12ca94752f1 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts @@ -191,8 +191,9 @@ export function getClientErrorObject( { error_type: ErrorTypeEnum.FRONTEND_NETWORK_ERROR, level: 'error', - message: + message: t( 'Network Error when attempting to load resource. Please check your internet connection and try again.', + ), extra: {}, }, ], From c12852f989fbbaba8f031a152b8579052dc62eba Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 20:31:44 +0300 Subject: [PATCH 06/18] chore(ErrorModal): change header color --- superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx index e441505beb524..6de4c5657e02a 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx @@ -83,6 +83,7 @@ const ErrorModal = styled(Modal)<{ level: ErrorLevel }>` display: flex; align-items: center; font-size: ${({ theme }) => theme.typography.sizes.l}px; + color: ${({ level, theme }) => theme.colors[level].dark2}; } `; From c1f68006266465188b7892b31e9ee15c23e0be55 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 13 Jan 2025 20:34:16 +0300 Subject: [PATCH 07/18] test(NetworkErrorMessage): add some basic test for network error --- .../ErrorMessage/NetworkErrorMessage.test.tsx | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx diff --git a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx new file mode 100644 index 0000000000000..96935a6e9b4d5 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx @@ -0,0 +1,61 @@ +/** + * 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 { ErrorLevel, ErrorSource, ErrorTypeEnum } from '@superset-ui/core'; +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; +import NetworkErrorMessage from './NetworkErrorMessage'; + +jest.mock( + 'src/components/Icons/Icon', + () => + ({ fileName }: { fileName: string }) => ( + + ), +); + +const mockedProps = { + error: { + error_type: ErrorTypeEnum.FRONTEND_NETWORK_ERROR, + extra: {}, + level: 'error' as ErrorLevel, + message: 'Error message', + }, + source: 'dashboard' as ErrorSource, + subtitle: 'Error message', +}; + +test('should render', () => { + const nullExtraProps = { + ...mockedProps, + error: { + ...mockedProps.error, + extra: null, + }, + }; + const { container } = render(); + expect(container).toBeInTheDocument(); +}); + +test('should render the error message', () => { + render(, { useRedux: true }); + const button = screen.getByText('See more'); + userEvent.click(button); + expect(screen.getByText('Error message')).toBeInTheDocument(); +}); From 60b341ecc21a5babb4f07ea5a247b794563daa50 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Tue, 14 Jan 2025 12:29:22 +0300 Subject: [PATCH 08/18] test(getClientErrorObject): add test for coverage --- .../test/query/getClientErrorObject.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts b/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts index 1abbf5c3065f9..c4ab0e0447d31 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts @@ -125,6 +125,14 @@ test('Handles TypeError Response', async () => { expect(errorObj).toMatchObject({ error: 'Network error' }); }); +test('Handles TypeError NetworkError Response', async () => { + const error = new TypeError('NetworkError'); + + // @ts-ignore + const errorObj = await getClientErrorObject(error); + expect(errorObj).toMatchObject({ error: 'Network error' }); +}); + test('Handles timeout error', async () => { const errorObj = await getClientErrorObject({ timeout: 1000, From 7038026d04ae1ba83b93dacabe71de76697db924 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 20:25:17 +0300 Subject: [PATCH 09/18] chore(ErrorMessage): cleanup --- ...x => FrontendNetworkErrorMessage.test.tsx} | 11 ++--- .../FrontendNetworkErrorMessage.tsx | 7 ++- .../ErrorMessage/NetworkErrorMessage.tsx | 43 ------------------- .../src/setup/setupErrorMessages.ts | 4 -- 4 files changed, 12 insertions(+), 53 deletions(-) rename superset-frontend/src/components/ErrorMessage/{NetworkErrorMessage.test.tsx => FrontendNetworkErrorMessage.test.tsx} (85%) delete mode 100644 superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx diff --git a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx similarity index 85% rename from superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx rename to superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx index 96935a6e9b4d5..6dce3f7492687 100644 --- a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.test.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx @@ -20,7 +20,7 @@ import { ErrorLevel, ErrorSource, ErrorTypeEnum } from '@superset-ui/core'; import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import NetworkErrorMessage from './NetworkErrorMessage'; +import FrontendNetworkErrorMessage from './FrontendNetworkErrorMessage'; jest.mock( 'src/components/Icons/Icon', @@ -49,13 +49,14 @@ test('should render', () => { extra: null, }, }; - const { container } = render(); + const { container } = render( + , + ); expect(container).toBeInTheDocument(); }); test('should render the error message', () => { - render(, { useRedux: true }); - const button = screen.getByText('See more'); - userEvent.click(button); + render(, { useRedux: true }); + userEvent.click(screen.getByRole('button')); expect(screen.getByText('Error message')).toBeInTheDocument(); }); diff --git a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx index 07d8567caabbc..278babbf8f3c0 100644 --- a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx @@ -27,7 +27,12 @@ function FrontendNetworkErrorMessage({ }: ErrorMessageComponentProps) { const { level, message } = error; return ( - + ); } export default FrontendNetworkErrorMessage; diff --git a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx deleted file mode 100644 index ceb7c5ce194e1..0000000000000 --- a/superset-frontend/src/components/ErrorMessage/NetworkErrorMessage.tsx +++ /dev/null @@ -1,43 +0,0 @@ -/** - * 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 { t } from '@superset-ui/core'; - -import { ErrorMessageComponentProps } from './types'; -import ErrorAlert from './ErrorAlert'; - -function NetworkErrorMessage({ - error, - source = 'dashboard', - subtitle, -}: ErrorMessageComponentProps) { - const { level, message } = error; - - return ( - - ); -} - -export default NetworkErrorMessage; diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts index 5f9b822403fe5..6f8183ba1cf3c 100644 --- a/superset-frontend/src/setup/setupErrorMessages.ts +++ b/superset-frontend/src/setup/setupErrorMessages.ts @@ -168,9 +168,5 @@ export default function setupErrorMessages() { ErrorTypeEnum.RESULT_TOO_LARGE_ERROR, DatabaseErrorMessage, ); - errorMessageComponentRegistry.registerValue( - ErrorTypeEnum.FRONTEND_NETWORK_ERROR, - NetworkErrorMessage, - ); setupErrorMessagesExtra(); } From d22626fa6081fe0d2f28f67c0c6cf18b7ae7ba16 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 20:34:27 +0300 Subject: [PATCH 10/18] chore(compact): add compact prop to ErrorWithStackTrace and network error --- .../components/ErrorMessage/ErrorMessageWithStackTrace.tsx | 4 ++++ .../components/ErrorMessage/FrontendNetworkErrorMessage.tsx | 3 ++- superset-frontend/src/components/ErrorMessage/types.ts | 1 + .../nativeFilters/FilterBar/FilterControls/FilterValue.tsx | 1 + 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx index 6b457df5255fb..9d364245748ba 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx @@ -35,6 +35,7 @@ type Props = { descriptionDetails?: ReactNode; errorMitigationFunction?: () => void; fallback?: ReactNode; + compact?: boolean; }; export default function ErrorMessageWithStackTrace({ @@ -48,6 +49,7 @@ export default function ErrorMessageWithStackTrace({ description, descriptionDetails, fallback, + compact, }: Props) { // Check if a custom error message component was registered for this message if (error) { @@ -57,6 +59,7 @@ export default function ErrorMessageWithStackTrace({ if (ErrorMessageComponent) { return ( ); } diff --git a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx index 278babbf8f3c0..303735ca49d69 100644 --- a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.tsx @@ -24,11 +24,12 @@ import ErrorAlert from './ErrorAlert'; function FrontendNetworkErrorMessage({ error, subtitle, + compact, }: ErrorMessageComponentProps) { const { level, message } = error; return ( | null> = error: SupersetError; source?: ErrorSource; subtitle?: ReactNode; + compact?: boolean; }; export type ErrorMessageComponent = ComponentType; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 29059b819902c..0607f1c31b157 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -306,6 +306,7 @@ const FilterValue: FC = ({ return ( Date: Fri, 24 Jan 2025 20:44:08 +0300 Subject: [PATCH 11/18] chore(errors): compact on chart --- superset-frontend/src/components/Chart/ChartErrorMessage.tsx | 2 +- .../src/components/ErrorMessage/BasicErrorAlert.tsx | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx index 0454f1fe464d9..bf1cbce958371 100644 --- a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx +++ b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx @@ -42,5 +42,5 @@ export const ChartErrorMessage: FC = ({ chartId, error, ...props }) => { extra: { ...error.extra, owners }, }; - return ; + return ; }; diff --git a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx index 1dcb5393c231a..45124c0bc4981 100644 --- a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx +++ b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx @@ -23,15 +23,13 @@ import { Tooltip } from '../Tooltip'; const StyledContainer = styled.div<{ level: ErrorLevel }>` display: flex; flex-direction: row; - align-items: center; background-color: ${({ level, theme }) => theme.colors[level].light2}; border-radius: ${({ theme }) => theme.borderRadius}px; border: 1px solid ${({ level, theme }) => theme.colors[level].base}; color: ${({ level, theme }) => theme.colors[level].dark2}; - padding: ${({ theme }) => theme.gridUnit * 1}px; + padding: ${({ theme }) => theme.gridUnit * 2}px; margin-bottom: ${({ theme }) => theme.gridUnit}px; width: 100%; - min-width: max-content; `; const StyledContent = styled.div` From 5f7a0ee5bf619ba72665dd73cab5f5580c7170d2 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 20:48:47 +0300 Subject: [PATCH 12/18] chore: remove unneeded changes --- .../src/query/getClientErrorObject.ts | 25 ------------------- .../test/query/getClientErrorObject.test.ts | 8 ------ .../ErrorMessage/BasicErrorAlert.tsx | 20 --------------- .../FilterBar/FilterControls/FilterValue.tsx | 1 - 4 files changed, 54 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts index 8e12ca94752f1..4c57745671c98 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts @@ -176,31 +176,6 @@ export function getClientErrorObject( return; } - if ( - (response instanceof TypeError && - response.message === 'Failed to fetch') || - (response instanceof TypeError && - response.message.includes('NetworkError')) - ) { - resolve({ - error: t('Network error'), - message: t( - 'Network Error when attempting to load resource. Please check your internet connection and try again.', - ), - errors: [ - { - error_type: ErrorTypeEnum.FRONTEND_NETWORK_ERROR, - level: 'error', - message: t( - 'Network Error when attempting to load resource. Please check your internet connection and try again.', - ), - extra: {}, - }, - ], - }); - return; - } - if ( 'timeout' in response && 'statusText' in response && diff --git a/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts b/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts index c4ab0e0447d31..1abbf5c3065f9 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/getClientErrorObject.test.ts @@ -125,14 +125,6 @@ test('Handles TypeError Response', async () => { expect(errorObj).toMatchObject({ error: 'Network error' }); }); -test('Handles TypeError NetworkError Response', async () => { - const error = new TypeError('NetworkError'); - - // @ts-ignore - const errorObj = await getClientErrorObject(error); - expect(errorObj).toMatchObject({ error: 'Network error' }); -}); - test('Handles timeout error', async () => { const errorObj = await getClientErrorObject({ timeout: 1000, diff --git a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx index 45124c0bc4981..39ae748970072 100644 --- a/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx +++ b/superset-frontend/src/components/ErrorMessage/BasicErrorAlert.tsx @@ -18,7 +18,6 @@ */ import { ErrorLevel, styled, useTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; -import { Tooltip } from '../Tooltip'; const StyledContainer = styled.div<{ level: ErrorLevel }>` display: flex; @@ -47,35 +46,16 @@ interface BasicErrorAlertProps { title: string; body: string; level?: ErrorLevel; - bodyOnTooltip?: boolean; } export default function BasicErrorAlert({ body, level = 'error', title, - bodyOnTooltip, }: BasicErrorAlertProps) { const theme = useTheme(); const iconColor = theme.colors[level].base; - if (bodyOnTooltip) { - return ( - - - {level === 'error' ? ( - - ) : ( - - )} - - {title} - - - - ); - } - return ( {level === 'error' ? ( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 0607f1c31b157..6c3c594e88c9a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -312,7 +312,6 @@ const FilterValue: FC = ({ title={t('Cannot load filter')} body={error.error} level="error" - bodyOnTooltip /> } /> From 19abff92933c2b7d45fca3f8f440a1d9c7278e93 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 20:50:00 +0300 Subject: [PATCH 13/18] chore: remove unneeded changes --- .../superset-ui-core/src/query/getClientErrorObject.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts index 4c57745671c98..dd26aec6192d7 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/getClientErrorObject.ts @@ -176,6 +176,16 @@ export function getClientErrorObject( return; } + if ( + response instanceof TypeError && + response.message === 'Failed to fetch' + ) { + resolve({ + error: t('Network error'), + }); + return; + } + if ( 'timeout' in response && 'statusText' in response && From 255c6d45057a9e03e2f6e6a299dfecfe136e235b Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 21:21:36 +0300 Subject: [PATCH 14/18] fix(filter): only filter value error compact --- .../src/components/Chart/ChartErrorMessage.tsx | 2 +- .../FilterBar/FilterControls/FilterValue.tsx | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx index bf1cbce958371..0454f1fe464d9 100644 --- a/superset-frontend/src/components/Chart/ChartErrorMessage.tsx +++ b/superset-frontend/src/components/Chart/ChartErrorMessage.tsx @@ -42,5 +42,5 @@ export const ChartErrorMessage: FC = ({ chartId, error, ...props }) => { extra: { ...error.extra, owners }, }; - return ; + return ; }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 6c3c594e88c9a..259e0de1943dc 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -60,6 +60,7 @@ import { FilterControlProps } from './types'; import { getFormData } from '../../utils'; import { useFilterDependencies } from './state'; import { useFilterOutlined } from '../useFilterOutlined'; +import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert'; const HEIGHT = 32; @@ -308,10 +309,11 @@ const FilterValue: FC = ({ error={error.errors?.[0]} compact fallback={ - } /> From d062294476abed68f28cb29a061fff1f9dd34b11 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Fri, 24 Jan 2025 21:33:27 +0300 Subject: [PATCH 15/18] chore(FilterValue): organize imports --- .../nativeFilters/FilterBar/FilterControls/FilterValue.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 259e0de1943dc..1757cb6d5fc3a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -45,7 +45,6 @@ import { useDispatch, useSelector } from 'react-redux'; import { isEqual, isEqualWith } from 'lodash'; import { getChartDataRequest } from 'src/components/Chart/chartAction'; import Loading from 'src/components/Loading'; -import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert'; import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import { waitForAsyncData } from 'src/middleware/asyncEvent'; import { FilterBarOrientation, RootState } from 'src/dashboard/types'; @@ -55,12 +54,12 @@ import { } from 'src/dashboard/actions/dashboardState'; import { RESPONSIVE_WIDTH } from 'src/filters/components/common'; import { FAST_DEBOUNCE } from 'src/constants'; +import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert'; import { dispatchHoverAction, dispatchFocusAction } from './utils'; import { FilterControlProps } from './types'; import { getFormData } from '../../utils'; import { useFilterDependencies } from './state'; import { useFilterOutlined } from '../useFilterOutlined'; -import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert'; const HEIGHT = 32; From e1ffb388394359bd4cc5f5195d6f06ece6fea59d Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 27 Jan 2025 13:00:12 +0300 Subject: [PATCH 16/18] test(NetworkError): fix and add some tests --- .../ErrorMessage/FrontendNetworkErrorMessage.test.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx index 6dce3f7492687..a727b076cf91d 100644 --- a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx @@ -18,8 +18,8 @@ */ import { ErrorLevel, ErrorSource, ErrorTypeEnum } from '@superset-ui/core'; -import { render, screen } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; +import { render, screen } from 'spec/helpers/testing-library'; import FrontendNetworkErrorMessage from './FrontendNetworkErrorMessage'; jest.mock( @@ -57,6 +57,15 @@ test('should render', () => { test('should render the error message', () => { render(, { useRedux: true }); + // userEvent.click(screen.getByRole('button')); + expect(screen.getByText('Error message')).toBeInTheDocument(); +}); + +test("should render error message in compact mode if 'compact' is true", () => { + render(, { + useRedux: true, + }); + expect(screen.queryByText('Error message')).not.toBeInTheDocument(); userEvent.click(screen.getByRole('button')); expect(screen.getByText('Error message')).toBeInTheDocument(); }); From 19681899b3b97dc757488d210ca3ac452981a670 Mon Sep 17 00:00:00 2001 From: Mehmet Salih Yavuz Date: Mon, 27 Jan 2025 13:03:27 +0300 Subject: [PATCH 17/18] chore(ErrorMessage): remove story covered by another pr --- .../ErrorMessage/ErrorMessage.stories.tsx | 72 ------------------- 1 file changed, 72 deletions(-) delete mode 100644 superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx deleted file mode 100644 index 6519a7ba546b2..0000000000000 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessage.stories.tsx +++ /dev/null @@ -1,72 +0,0 @@ -/** - * 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 BasicErrorAlert from './BasicErrorAlert'; -import ErrorAlert from './ErrorAlert'; - -export default { - title: 'ErrorMessage', -}; - -export const InteractiveBasicErrorAlert = (args: any) => ( - -); - -InteractiveBasicErrorAlert.argTypes = { - level: { - control: { - type: 'select', - }, - options: ['error', 'warning', 'info'], - }, -}; - -InteractiveBasicErrorAlert.args = { - title: 'Title', - body: 'Body', - level: 'error', - bodyOnTooltip: false, -}; - -export const InteractiveErrorAlert = (args: any) => ; - -InteractiveErrorAlert.argTypes = { - level: { - control: { - type: 'select', - }, - options: ['error', 'warning', 'info'], - }, - source: { - control: { - type: 'select', - }, - options: ['dashboard', 'explore', 'sqllab', 'crud'], - }, -}; - -InteractiveErrorAlert.args = { - title: 'Title', - copyText: 'Copy error message', - level: 'error', - source: 'dashboard', - subtitle: 'Subtitle', - body: 'Body', - description: 'Description', -}; From a34ecbfebb9c1ba8228a1be24774bddee59ebd01 Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:46:35 +0100 Subject: [PATCH 18/18] Update superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx --- .../components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx index a727b076cf91d..8e53f5f02bd35 100644 --- a/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx +++ b/superset-frontend/src/components/ErrorMessage/FrontendNetworkErrorMessage.test.tsx @@ -57,7 +57,6 @@ test('should render', () => { test('should render the error message', () => { render(, { useRedux: true }); - // userEvent.click(screen.getByRole('button')); expect(screen.getByText('Error message')).toBeInTheDocument(); });