From 5fa46804473ad1b3909aaef4bc1d4e729c048882 Mon Sep 17 00:00:00 2001 From: Erik Ritter Date: Mon, 20 Jul 2020 15:32:17 -0700 Subject: [PATCH] feat: update timeout error UX (#10274) --- docs/index.rst | 1 + docs/issue_code_reference.rst | 39 +++ superset-frontend/images/icons/close.svg | 21 ++ .../javascripts/chart/chartActions_spec.js | 4 +- .../javascripts/chart/chartReducers_spec.js | 16 +- .../src/SqlLab/components/ResultSet.tsx | 13 +- superset-frontend/src/SqlLab/main.less | 4 + superset-frontend/src/chart/Chart.jsx | 19 +- superset-frontend/src/chart/chartAction.js | 20 +- superset-frontend/src/chart/chartReducer.js | 15 -- .../ErrorMessageWithStackTrace.tsx | 6 +- .../src/components/ErrorMessage/IssueCode.tsx | 39 +++ .../ErrorMessage/TimeoutErrorMessage.tsx | 248 ++++++++++++++++++ .../src/components/ErrorMessage/types.ts | 16 +- superset-frontend/src/components/Icon.tsx | 21 +- .../components/gridComponents/Chart.jsx | 1 + .../src/dashboard/reducers/getInitialState.js | 1 + .../src/dashboard/util/propShapes.jsx | 1 + .../explore/components/ExploreChartPanel.jsx | 1 + .../src/setup/setupErrorMessages.ts | 15 +- superset-frontend/src/utils/common.js | 2 + .../src/utils/getClientErrorObject.ts | 37 ++- superset/errors.py | 28 ++ superset/exceptions.py | 16 +- superset/models/slice.py | 9 +- superset/utils/core.py | 8 +- superset/views/base.py | 9 +- superset/views/core.py | 7 +- tests/core_tests.py | 5 +- 29 files changed, 557 insertions(+), 65 deletions(-) create mode 100644 docs/issue_code_reference.rst create mode 100644 superset-frontend/images/icons/close.svg create mode 100644 superset-frontend/src/components/ErrorMessage/IssueCode.tsx create mode 100644 superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx diff --git a/docs/index.rst b/docs/index.rst index 6a592fe709c..d1f0e5813a4 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -164,6 +164,7 @@ Contents gallery druid misc + issue_code_reference faq diff --git a/docs/issue_code_reference.rst b/docs/issue_code_reference.rst new file mode 100644 index 00000000000..ef89d1e51d3 --- /dev/null +++ b/docs/issue_code_reference.rst @@ -0,0 +1,39 @@ +.. 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. + +Issue Code Reference +==================== + +This page lists issue codes that may be displayed in Superset and provides additional context. + +Issue 1000 +"""""""""" + +.. code-block:: text + + The datasource is too large to query. + +It's likely your datasource has grown too large to run the current query, and is timing out. You can resolve this by reducing the size of your datasource or by modifying your query to only process a subset of your data. + +Issue 1001 +"""""""""" + +.. code-block:: text + + The database is under an unusual load. + +Your query may have timed out because of unusually high load on the database engine. You can make your query simpler, or wait until the database is under less load and try again. diff --git a/superset-frontend/images/icons/close.svg b/superset-frontend/images/icons/close.svg new file mode 100644 index 00000000000..33448814eff --- /dev/null +++ b/superset-frontend/images/icons/close.svg @@ -0,0 +1,21 @@ + + + + diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js b/superset-frontend/spec/javascripts/chart/chartActions_spec.js index a5258854c1c..13710744daf 100644 --- a/superset-frontend/spec/javascripts/chart/chartActions_spec.js +++ b/superset-frontend/spec/javascripts/chart/chartActions_spec.js @@ -156,7 +156,7 @@ describe('chart actions', () => { }); }); - it('should CHART_UPDATE_TIMEOUT action upon query timeout', () => { + it('should dispatch CHART_UPDATE_FAILED action upon query timeout', () => { const unresolvingPromise = new Promise(() => {}); fetchMock.post(MOCK_URL, () => unresolvingPromise, { overwriteRoutes: true, @@ -169,7 +169,7 @@ describe('chart actions', () => { // chart update, trigger query, update form data, fail expect(fetchMock.calls(MOCK_URL)).toHaveLength(1); expect(dispatch.callCount).toBe(5); - expect(dispatch.args[4][0].type).toBe(actions.CHART_UPDATE_TIMEOUT); + expect(dispatch.args[4][0].type).toBe(actions.CHART_UPDATE_FAILED); setupDefaultFetchMock(); }); }); diff --git a/superset-frontend/spec/javascripts/chart/chartReducers_spec.js b/superset-frontend/spec/javascripts/chart/chartReducers_spec.js index 0ecd7abcbe3..f72ca40b85d 100644 --- a/superset-frontend/spec/javascripts/chart/chartReducers_spec.js +++ b/superset-frontend/spec/javascripts/chart/chartReducers_spec.js @@ -40,7 +40,21 @@ describe('chart reducers', () => { it('should update endtime on timeout', () => { const newState = chartReducer( charts, - actions.chartUpdateTimeout('timeout', 60, chartKey), + actions.chartUpdateFailed( + { + statusText: 'timeout', + error: 'Request timed out', + errors: [ + { + error_type: 'FRONTEND_TIMEOUT_ERROR', + extra: { timeout: 1 }, + level: 'error', + message: 'Request timed out', + }, + ], + }, + chartKey, + ), ); expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0); expect(newState[chartKey].chartStatus).toEqual('failed'); diff --git a/superset-frontend/src/SqlLab/components/ResultSet.tsx b/superset-frontend/src/SqlLab/components/ResultSet.tsx index dd2dd6f0079..db154fba431 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet.tsx @@ -217,11 +217,14 @@ export default class ResultSet extends React.PureComponent< return Query was stopped; } else if (query.state === 'failed') { return ( - +
+ +
); } else if (query.state === 'success' && query.ctas) { const { tempSchema, tempTable } = query; diff --git a/superset-frontend/src/SqlLab/main.less b/superset-frontend/src/SqlLab/main.less index 7b3fb9c764e..b09884d8fcf 100644 --- a/superset-frontend/src/SqlLab/main.less +++ b/superset-frontend/src/SqlLab/main.less @@ -409,6 +409,10 @@ div.tablePopover { padding-right: 8px; } +.result-set-error-message { + padding-top: 16px; +} + .filterable-table-container { margin-top: 48px; } diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 956230446ee..ff5a5707655 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -49,6 +49,7 @@ const propTypes = { timeout: PropTypes.number, vizType: PropTypes.string.isRequired, triggerRender: PropTypes.bool, + owners: PropTypes.arrayOf(PropTypes.string), // state chartAlert: PropTypes.string, chartStatus: PropTypes.string, @@ -139,12 +140,26 @@ class Chart extends React.PureComponent { } renderErrorMessage() { - const { chartAlert, chartStackTrace, queryResponse } = this.props; + const { + chartAlert, + chartStackTrace, + dashboardId, + owners, + queryResponse, + } = this.props; + + const error = queryResponse?.errors?.[0]; + if (error) { + const extra = error.extra || {}; + extra.owners = owners; + error.extra = extra; + } return ( ); diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js index 67f83b59c73..1be26264a3b 100644 --- a/superset-frontend/src/chart/chartAction.js +++ b/superset-frontend/src/chart/chartAction.js @@ -62,11 +62,6 @@ export function chartUpdateStopped(key) { return { type: CHART_UPDATE_STOPPED, key }; } -export const CHART_UPDATE_TIMEOUT = 'CHART_UPDATE_TIMEOUT'; -export function chartUpdateTimeout(statusText, timeout, key) { - return { type: CHART_UPDATE_TIMEOUT, statusText, timeout, key }; -} - export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED'; export function chartUpdateFailed(queryResponse, key) { return { type: CHART_UPDATE_FAILED, queryResponse, key }; @@ -391,19 +386,16 @@ export function exploreJSON( }), ); }; - - if (response.statusText === 'timeout') { - appendErrorLog('timeout'); - return dispatch( - chartUpdateTimeout(response.statusText, timeout, key), - ); - } else if (response.name === 'AbortError') { + if (response.name === 'AbortError') { appendErrorLog('abort'); return dispatch(chartUpdateStopped(key)); } return getClientErrorObject(response).then(parsedResponse => { - // query is processed, but error out. - appendErrorLog(parsedResponse.error, parsedResponse.is_cached); + if (response.statusText === 'timeout') { + appendErrorLog('timeout'); + } else { + appendErrorLog(parsedResponse.error, parsedResponse.is_cached); + } return dispatch(chartUpdateFailed(parsedResponse, key)); }); }); diff --git a/superset-frontend/src/chart/chartReducer.js b/superset-frontend/src/chart/chartReducer.js index d3e22365cce..c50c0074b03 100644 --- a/superset-frontend/src/chart/chartReducer.js +++ b/superset-frontend/src/chart/chartReducer.js @@ -85,21 +85,6 @@ export default function chartReducer(charts = {}, action) { ), }; }, - [actions.CHART_UPDATE_TIMEOUT](state) { - return { - ...state, - chartStatus: 'failed', - chartAlert: `${t('Query timeout')} - ${t( - `visualization queries are set to timeout at ${action.timeout} seconds. `, - )}${t( - 'Perhaps your data has grown, your database is under unusual load, ' + - 'or you are simply querying a data source that is too large ' + - 'to be processed within the timeout range. ' + - 'If that is the case, we recommend that you summarize your data further.', - )}`, - chartUpdateEndTime: now(), - }; - }, [actions.CHART_UPDATE_FAILED](state) { return { ...state, diff --git a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx index 5f3f6794e78..04befca0168 100644 --- a/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx +++ b/superset-frontend/src/components/ErrorMessage/ErrorMessageWithStackTrace.tsx @@ -22,13 +22,14 @@ import { Alert, Collapse } from 'react-bootstrap'; import { t } from '@superset-ui/translation'; import getErrorMessageComponentRegistry from './getErrorMessageComponentRegistry'; -import { SupersetError } from './types'; +import { SupersetError, ErrorSource } from './types'; type Props = { error?: SupersetError; link?: string; message?: string; stackTrace?: string; + source?: ErrorSource; }; export default function ErrorMessageWithStackTrace({ @@ -36,6 +37,7 @@ export default function ErrorMessageWithStackTrace({ message, link, stackTrace, + source, }: Props) { const [showStackTrace, setShowStackTrace] = useState(false); @@ -45,7 +47,7 @@ export default function ErrorMessageWithStackTrace({ error.error_type, ); if (ErrorMessageComponent) { - return ; + return ; } } diff --git a/superset-frontend/src/components/ErrorMessage/IssueCode.tsx b/superset-frontend/src/components/ErrorMessage/IssueCode.tsx new file mode 100644 index 00000000000..37543415f29 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/IssueCode.tsx @@ -0,0 +1,39 @@ +/** + * 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 React from 'react'; + +interface IssueCodeProps { + code: number; + message: string; +} + +export default function IssueCode({ code, message }: IssueCodeProps) { + return ( + <> + {message}{' '} + + + + + ); +} diff --git a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx new file mode 100644 index 00000000000..2b7ff01bb8d --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx @@ -0,0 +1,248 @@ +/** + * 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 React, { useState } from 'react'; +import { Modal } from 'react-bootstrap'; +import { styled, supersetTheme } from '@superset-ui/style'; +import { t, tn } from '@superset-ui/translation'; + +import { noOp } from 'src/utils/common'; +import Icon from '../Icon'; +import Button from '../../views/datasetList/Button'; +import { ErrorMessageComponentProps } from './types'; +import CopyToClipboard from '../CopyToClipboard'; +import IssueCode from './IssueCode'; + +const ErrorAlert = styled.div` + align-items: center; + background-color: ${({ theme }) => theme.colors.error.light2}; + border-radius: ${({ theme }) => theme.borderRadius}px; + border: 1px solid ${({ theme }) => theme.colors.error.base}; + color: ${({ theme }) => theme.colors.error.dark2}; + padding: ${({ theme }) => 2 * theme.gridUnit}px; + width: 100%; + + .top-row { + display: flex; + justify-content: space-between; + } + + .error-body { + padding-top: ${({ theme }) => theme.gridUnit}px; + padding-left: ${({ theme }) => 8 * theme.gridUnit}px; + } + + .icon { + margin-right: ${({ theme }) => 2 * theme.gridUnit}px; + } + + .link { + color: ${({ theme }) => theme.colors.error.dark2}; + text-decoration: underline; + } +`; + +const ErrorModal = styled(Modal)` + color: ${({ theme }) => theme.colors.error.dark2}; + + .icon { + margin-right: ${({ theme }) => 2 * theme.gridUnit}px; + } + + .header { + align-items: center; + background-color: ${({ theme }) => theme.colors.error.light2}; + display: flex; + justify-content: space-between; + font-size: ${({ theme }) => theme.typography.sizes.l}px; + + // Remove clearfix hack as Superset is only used on modern browsers + ::before, + ::after { + content: unset; + } + } +`; + +const LeftSideContent = styled.div` + align-items: center; + display: flex; +`; + +interface TimeoutErrorExtra { + issue_codes: { + code: number; + message: string; + }[]; + owners?: string[]; + timeout: number; +} + +function TimeoutErrorMessage({ + error, + source, +}: ErrorMessageComponentProps) { + const [isModalOpen, setIsModalOpen] = useState(false); + const [isMessageExpanded, setIsMessageExpanded] = useState(false); + const { extra } = error; + + const isVisualization = (['dashboard', 'explore'] as ( + | string + | undefined + )[]).includes(source); + + const isExpandable = (['explore', 'sqllab'] as ( + | string + | undefined + )[]).includes(source); + + const title = isVisualization + ? tn( + 'We’re having trouble loading this visualization. Queries are set to timeout after %s second.', + 'We’re having trouble loading this visualization. Queries are set to timeout after %s seconds.', + extra.timeout, + extra.timeout, + ) + : tn( + 'We’re having trouble loading these results. Queries are set to timeout after %s second.', + 'We’re having trouble loading these results. Queries are set to timeout after %s seconds.', + extra.timeout, + extra.timeout, + ); + + const message = ( + <> +

+ {t('This may be triggered by:')} +
+ {extra.issue_codes + .map(issueCode => ) + .reduce((prev, curr) => [prev,
, curr])} +

+ {isVisualization && extra.owners && ( + <> +
+

+ {tn( + 'Please reach out to the Chart Owner for assistance.', + 'Please reach out to the Chart Owners for assistance.', + extra.owners.length, + )} +

+

+ {tn( + 'Chart Owner: %s', + 'Chart Owners: %s', + extra.owners.length, + extra.owners.join(', '), + )} +

+ + )} + + ); + + const copyText = `${title} +${t('This may be triggered by:')} +${extra.issue_codes.map(issueCode => issueCode.message).join('\n')}`; + + return ( + +
+ + + {t('Timeout Error')} + + {!isExpandable && ( + setIsModalOpen(true)}> + {t('See More')} + + )} +
+ {isExpandable ? ( +
+

{title}

+ {!isMessageExpanded && ( + setIsMessageExpanded(true)} + > + {t('See More')} + + )} + {isMessageExpanded && ( + <> +
+ {message} + setIsMessageExpanded(false)} + > + {t('See Less')} + + + )} +
+ ) : ( + setIsModalOpen(false)}> + + + +
{t('Timeout Error')}
+ + setIsModalOpen(false)} + > + + + + +

{title}

+
+ {message} + + + {t('Copy Message')}} + /> + + + + )} + + ); +} + +export default TimeoutErrorMessage; diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index 5834c2c3356..e5b1b682190 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -37,6 +37,9 @@ export const ErrorTypeEnum = { TABLE_SECURITY_ACCESS_ERROR: 'TABLE_SECURITY_ACCESS_ERROR', DATASOURCE_SECURITY_ACCESS_ERROR: 'DATASOURCE_SECURITY_ACCESS_ERROR', MISSING_OWNERSHIP_ERROR: 'MISSING_OWNERSHIP_ERROR', + + // Other errors + BACKEND_TIMEOUT_ERROR: 'BACKEND_TIMEOUT_ERROR', } as const; type ValueOf = T[keyof T]; @@ -46,15 +49,20 @@ export type ErrorType = ValueOf; // Keep in sync with superset/views/errors.py export type ErrorLevel = 'info' | 'warning' | 'error'; -export type SupersetError = { +export type ErrorSource = 'dashboard' | 'explore' | 'sqllab'; + +export type SupersetError | null> = { error_type: ErrorType; - extra: Record | null; + extra: ExtraType; level: ErrorLevel; message: string; }; -export type ErrorMessageComponentProps = { - error: SupersetError; +export type ErrorMessageComponentProps< + ExtraType = Record | null +> = { + error: SupersetError; + source?: ErrorSource; }; export type ErrorMessageComponent = React.ComponentType< diff --git a/superset-frontend/src/components/Icon.tsx b/superset-frontend/src/components/Icon.tsx index e7dfe8da8b9..1c8541a31ec 100644 --- a/superset-frontend/src/components/Icon.tsx +++ b/superset-frontend/src/components/Icon.tsx @@ -17,12 +17,12 @@ * under the License. */ import React, { SVGProps } from 'react'; -import styled from '@superset-ui/style'; import { ReactComponent as CancelXIcon } from 'images/icons/cancel-x.svg'; import { ReactComponent as CheckIcon } from 'images/icons/check.svg'; import { ReactComponent as CheckboxHalfIcon } from 'images/icons/checkbox-half.svg'; import { ReactComponent as CheckboxOffIcon } from 'images/icons/checkbox-off.svg'; import { ReactComponent as CheckboxOnIcon } from 'images/icons/checkbox-on.svg'; +import { ReactComponent as CloseIcon } from 'images/icons/close.svg'; import { ReactComponent as CompassIcon } from 'images/icons/compass.svg'; import { ReactComponent as DatasetPhysicalIcon } from 'images/icons/dataset_physical.svg'; import { ReactComponent as DatasetVirtualIcon } from 'images/icons/dataset_virtual.svg'; @@ -35,12 +35,13 @@ import { ReactComponent as SortIcon } from 'images/icons/sort.svg'; import { ReactComponent as TrashIcon } from 'images/icons/trash.svg'; import { ReactComponent as WarningIcon } from 'images/icons/warning.svg'; -type Icon = +type IconName = | 'cancel-x' | 'check' | 'checkbox-half' | 'checkbox-off' | 'checkbox-on' + | 'close' | 'compass' | 'dataset-physical' | 'dataset-virtual' @@ -53,7 +54,10 @@ type Icon = | 'trash' | 'warning'; -const iconsRegistry: { [key in Icon]: React.ComponentType } = { +const iconsRegistry: Record< + IconName, + React.ComponentType> +> = { 'cancel-x': CancelXIcon, 'checkbox-half': CheckboxHalfIcon, 'checkbox-off': CheckboxOffIcon, @@ -63,6 +67,7 @@ const iconsRegistry: { [key in Icon]: React.ComponentType } = { 'sort-asc': SortAscIcon, 'sort-desc': SortDescIcon, check: CheckIcon, + close: CloseIcon, compass: CompassIcon, error: ErrorIcon, pencil: PencilIcon, @@ -72,14 +77,12 @@ const iconsRegistry: { [key in Icon]: React.ComponentType } = { warning: WarningIcon, }; interface IconProps extends SVGProps { - name: Icon; + name: IconName; } -const Icon = ({ name, ...rest }: IconProps) => { +const Icon = ({ name, color = '#666666', ...rest }: IconProps) => { const Component = iconsRegistry[name]; - return ; + return ; }; -export default styled(Icon)<{}>` - color: #666666; -`; +export default Icon; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 4a443f15a7e..96ea55770c4 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -336,6 +336,7 @@ class Chart extends React.Component { timeout={timeout} triggerQuery={chart.triggerQuery} vizType={slice.viz_type} + owners={slice.owners} /> diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js index 3d4e38a6c5e..4f77b070c71 100644 --- a/superset-frontend/src/dashboard/reducers/getInitialState.js +++ b/superset-frontend/src/dashboard/reducers/getInitialState.js @@ -134,6 +134,7 @@ export default function (bootstrapData) { datasource: slice.form_data.datasource, description: slice.description, description_markeddown: slice.description_markeddown, + owners: slice.owners, modified: slice.modified, changed_on: new Date(slice.changed_on).getTime(), }; diff --git a/superset-frontend/src/dashboard/util/propShapes.jsx b/superset-frontend/src/dashboard/util/propShapes.jsx index 6b41b8a490f..e1eb01cb27d 100644 --- a/superset-frontend/src/dashboard/util/propShapes.jsx +++ b/superset-frontend/src/dashboard/util/propShapes.jsx @@ -68,6 +68,7 @@ export const slicePropShape = PropTypes.shape({ viz_type: PropTypes.string.isRequired, description: PropTypes.string, description_markeddown: PropTypes.string, + owners: PropTypes.arrayOf(PropTypes.string).isRequired, }); export const filterIndicatorPropShape = PropTypes.shape({ diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx index 409ce5e8101..36975a03b61 100644 --- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx +++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx @@ -72,6 +72,7 @@ class ExploreChartPanel extends React.PureComponent { errorMessage={this.props.errorMessage} formData={this.props.form_data} onQuery={this.props.onQuery} + owners={this.props?.slice?.owners} queryResponse={chart.queryResponse} refreshOverlayVisible={this.props.refreshOverlayVisible} setControlValue={this.props.actions.setControlValue} diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts index 289b3cb0db3..269e7c57352 100644 --- a/superset-frontend/src/setup/setupErrorMessages.ts +++ b/superset-frontend/src/setup/setupErrorMessages.ts @@ -16,9 +16,22 @@ * specific language governing permissions and limitations * under the License. */ +import getErrorMessageComponentRegistry from 'src/components/ErrorMessage/getErrorMessageComponentRegistry'; +import { ErrorTypeEnum } from 'src/components/ErrorMessage/types'; +import TimeoutErrorMessage from 'src/components/ErrorMessage/TimeoutErrorMessage'; + import setupErrorMessagesExtra from './setupErrorMessagesExtra'; export default function setupErrorMessages() { - // TODO: Register error messages to the ErrorMessageComponentRegistry once implemented + const errorMessageComponentRegistry = getErrorMessageComponentRegistry(); + + errorMessageComponentRegistry.registerValue( + ErrorTypeEnum.FRONTEND_TIMEOUT_ERROR, + TimeoutErrorMessage, + ); + errorMessageComponentRegistry.registerValue( + ErrorTypeEnum.BACKEND_TIMEOUT_ERROR, + TimeoutErrorMessage, + ); setupErrorMessagesExtra(); } diff --git a/superset-frontend/src/utils/common.js b/superset-frontend/src/utils/common.js index 268182748ed..e43def8e352 100644 --- a/superset-frontend/src/utils/common.js +++ b/superset-frontend/src/utils/common.js @@ -136,3 +136,5 @@ export function applyFormattingToTabularData(data) { /* eslint-enable no-underscore-dangle */ })); } + +export const noOp = () => undefined; diff --git a/superset-frontend/src/utils/getClientErrorObject.ts b/superset-frontend/src/utils/getClientErrorObject.ts index 421aa01a57a..369e5c5b5ab 100644 --- a/superset-frontend/src/utils/getClientErrorObject.ts +++ b/superset-frontend/src/utils/getClientErrorObject.ts @@ -18,7 +18,10 @@ */ import { SupersetClientResponse } from '@superset-ui/connection'; import { t } from '@superset-ui/translation'; -import { SupersetError } from 'src/components/ErrorMessage/types'; +import { + SupersetError, + ErrorTypeEnum, +} from 'src/components/ErrorMessage/types'; import COMMON_ERR_MESSAGES from './errorMessages'; // The response always contains an error attribute, can contain anything from the @@ -84,6 +87,38 @@ export default function getClientErrorObject( resolve({ ...responseObject, error: errorText }); }); }); + } else if ( + 'statusText' in response && + response.statusText === 'timeout' + ) { + resolve({ + ...responseObject, + error: 'Request timed out', + errors: [ + { + error_type: ErrorTypeEnum.FRONTEND_TIMEOUT_ERROR, + extra: { + timeout: 1, + issue_codes: [ + { + code: 1000, + message: t( + 'Issue 1000 - The datasource is too large to query.', + ), + }, + { + code: 1001, + message: t( + 'Issue 1001 - The database is under an unusual load.', + ), + }, + ], + }, + level: 'error', + message: 'Request timed out', + }, + ], + }); } else { // fall back to Response.statusText or generic error of we cannot read the response const error = diff --git a/superset/errors.py b/superset/errors.py index 54eb0ed63b1..f2fda00bb86 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -19,6 +19,7 @@ from enum import Enum from typing import Any, Dict, Optional from dataclasses import dataclass +from flask_babel import gettext as _ class SupersetErrorType(str, Enum): @@ -46,6 +47,23 @@ class SupersetErrorType(str, Enum): DATASOURCE_SECURITY_ACCESS_ERROR = "DATASOURCE_SECURITY_ACCESS_ERROR" MISSING_OWNERSHIP_ERROR = "MISSING_OWNERSHIP_ERROR" + # Other errors + BACKEND_TIMEOUT_ERROR = "BACKEND_TIMEOUT_ERROR" + + +ERROR_TYPES_TO_ISSUE_CODES_MAPPING = { + SupersetErrorType.BACKEND_TIMEOUT_ERROR: [ + { + "code": 1000, + "message": _("Issue 1000 - The datasource is too large to query."), + }, + { + "code": 1001, + "message": _("Issue 1001 - The database is under an unusual load."), + }, + ] +} + class ErrorLevel(str, Enum): """ @@ -69,3 +87,13 @@ class SupersetError: error_type: SupersetErrorType level: ErrorLevel extra: Optional[Dict[str, Any]] = None + + def __post_init__(self) -> None: + """ + Mutates the extra params with user facing error codes that map to backend + errors. + """ + issue_codes = ERROR_TYPES_TO_ISSUE_CODES_MAPPING.get(self.error_type) + if issue_codes: + self.extra = self.extra or {} + self.extra.update({"issue_codes": issue_codes}) diff --git a/superset/exceptions.py b/superset/exceptions.py index c849d678b3f..3dca1dec661 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -18,7 +18,7 @@ from typing import Any, Dict, Optional from flask_babel import gettext as _ -from superset.errors import SupersetError +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType class SupersetException(Exception): @@ -37,7 +37,19 @@ class SupersetException(Exception): class SupersetTimeoutException(SupersetException): - pass + status = 408 + + def __init__( + self, + error_type: SupersetErrorType, + message: str, + level: ErrorLevel, + extra: Optional[Dict[str, Any]], + ) -> None: + super(SupersetTimeoutException, self).__init__(message) + self.error = SupersetError( + error_type=error_type, message=message, level=level, extra=extra + ) class SupersetSecurityException(SupersetException): diff --git a/superset/models/slice.py b/superset/models/slice.py index b8f1d935701..30f56cad8d0 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -177,17 +177,20 @@ class Slice( data["error"] = str(ex) return { "cache_timeout": self.cache_timeout, + "changed_on": self.changed_on.isoformat(), + "changed_on_humanized": self.changed_on_humanized, "datasource": self.datasource_name, "description": self.description, "description_markeddown": self.description_markeddown, "edit_url": self.edit_url, "form_data": self.form_data, + "modified": self.modified(), + "owners": [ + f"{owner.first_name} {owner.last_name}" for owner in self.owners + ], "slice_id": self.id, "slice_name": self.slice_name, "slice_url": self.slice_url, - "modified": self.modified(), - "changed_on_humanized": self.changed_on_humanized, - "changed_on": self.changed_on.isoformat(), } @property diff --git a/superset/utils/core.py b/superset/utils/core.py index c464d78d601..c6d895c4382 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -79,6 +79,7 @@ from sqlalchemy.engine.reflection import Inspector from sqlalchemy.sql.type_api import Variant from sqlalchemy.types import TEXT, TypeDecorator +from superset.errors import ErrorLevel, SupersetErrorType from superset.exceptions import ( CertificateException, SupersetException, @@ -617,7 +618,12 @@ class timeout: # pylint: disable=invalid-name self, signum: int, frame: Any ) -> None: logger.error("Process timed out") - raise SupersetTimeoutException(self.error_message) + raise SupersetTimeoutException( + error_type=SupersetErrorType.BACKEND_TIMEOUT_ERROR, + message=self.error_message, + level=ErrorLevel.ERROR, + extra={"timeout": self.seconds}, + ) def __enter__(self) -> None: try: diff --git a/superset/views/base.py b/superset/views/base.py index 0e17f5724ba..7aeae79bd3d 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -48,7 +48,11 @@ from superset import ( ) from superset.connectors.sqla import models from superset.errors import ErrorLevel, SupersetError, SupersetErrorType -from superset.exceptions import SupersetException, SupersetSecurityException +from superset.exceptions import ( + SupersetException, + SupersetSecurityException, + SupersetTimeoutException, +) from superset.models.helpers import ImportMixin from superset.translations.utils import get_language_pack from superset.typing import FlaskResponse @@ -176,6 +180,9 @@ def handle_api_exception( return json_errors_response( errors=[ex.error], status=ex.status, payload=ex.payload ) + except SupersetTimeoutException as ex: + logger.warning(ex) + return json_errors_response(errors=[ex.error], status=ex.status) except SupersetException as ex: logger.exception(ex) return json_error_response( diff --git a/superset/views/core.py b/superset/views/core.py index bce0996301b..42c2f97436c 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1906,7 +1906,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods ) except SupersetTimeoutException as ex: logger.exception(ex) - return json_error_response(timeout_msg) + return json_errors_response([ex.error]) except Exception as ex: # pylint: disable=broad-except return json_error_response(utils.error_msg_from_exception(ex)) @@ -2151,6 +2151,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods :param rendered_query: The rendered query (included templates) :param query: The query SQL (SQLAlchemy) object :return: A Flask Response + :raises: SupersetTimeoutException """ try: timeout = config["SQLLAB_TIMEOUT"] @@ -2177,6 +2178,9 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods ignore_nan=True, encoding=None, ) + except SupersetTimeoutException as ex: + # re-raise exception for api exception handler + raise ex except Exception as ex: # pylint: disable=broad-except logger.exception("Query %i: %s", query.id, str(ex)) return json_error_response(utils.error_msg_from_exception(ex)) @@ -2185,6 +2189,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods return json_success(payload) @has_access_api + @handle_api_exception @expose("/sql_json/", methods=["POST"]) @event_logger.log_this def sql_json(self) -> FlaskResponse: diff --git a/tests/core_tests.py b/tests/core_tests.py index 2e9ab4b36ea..2d18751cf40 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -315,10 +315,13 @@ class TestCore(SupersetTestCase): def test_slice_data(self): # slice data should have some required attributes self.login(username="admin") - slc = self.get_slice("Girls", db.session) + slc = self.get_slice( + slice_name="Girls", session=db.session, expunge_from_session=False + ) slc_data_attributes = slc.data.keys() assert "changed_on" in slc_data_attributes assert "modified" in slc_data_attributes + assert "owners" in slc_data_attributes def test_slices(self): # Testing by hitting the two supported end points for all slices