diff --git a/docs/index.rst b/docs/index.rst index 6a592fe70..d1f0e5813 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 000000000..ef89d1e51 --- /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 000000000..33448814e --- /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 a5258854c..13710744d 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 0ecd7abcb..f72ca40b8 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 dd2dd6f00..db154fba4 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 7b3fb9c76..b09884d8f 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 956230446..ff5a57076 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 67f83b59c..1be26264a 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 d3e22365c..c50c0074b 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 5f3f6794e..04befca01 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 000000000..37543415f --- /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 000000000..2b7ff01bb --- /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 5834c2c33..e5b1b6821 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 e7dfe8da8..1c8541a31 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 4a443f15a..96ea55770 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 3d4e38a6c..4f77b070c 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 6b41b8a49..e1eb01cb2 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 409ce5e81..36975a03b 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 289b3cb0d..269e7c573 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 268182748..e43def8e3 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 421aa01a5..369e5c5b5 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 54eb0ed63..f2fda00bb 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 c849d678b..3dca1dec6 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 b8f1d9357..30f56cad8 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 c464d78d6..c6d895c43 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 0e17f5724..7aeae79bd 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 bce099630..42c2f9743 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 2e9ab4b36..2d18751cf 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