You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by er...@apache.org on 2020/07/20 22:32:43 UTC
[incubator-superset] branch master updated: feat: update timeout
error UX (#10274)
This is an automated email from the ASF dual-hosted git repository.
erikrit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 5fa4680 feat: update timeout error UX (#10274)
5fa4680 is described below
commit 5fa46804473ad1b3909aaef4bc1d4e729c048882
Author: Erik Ritter <er...@airbnb.com>
AuthorDate: Mon Jul 20 15:32:17 2020 -0700
feat: update timeout error UX (#10274)
---
docs/index.rst | 1 +
docs/issue_code_reference.rst | 39 ++++
superset-frontend/images/icons/close.svg | 21 ++
.../spec/javascripts/chart/chartActions_spec.js | 4 +-
.../spec/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 --
.../ErrorMessage/ErrorMessageWithStackTrace.tsx | 6 +-
.../ErrorMessage/IssueCode.tsx} | 23 +-
.../ErrorMessage/TimeoutErrorMessage.tsx | 248 +++++++++++++++++++++
.../src/components/ErrorMessage/types.ts | 16 +-
superset-frontend/src/components/Icon.tsx | 21 +-
.../dashboard/components/gridComponents/Chart.jsx | 1 +
.../src/dashboard/reducers/getInitialState.js | 1 +
.../src/dashboard/util/propShapes.jsx | 1 +
.../src/explore/components/ExploreChartPanel.jsx | 1 +
superset-frontend/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, 537 insertions(+), 69 deletions(-)
diff --git a/docs/index.rst b/docs/index.rst
index 6a592fe..d1f0e58 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 0000000..ef89d1e
--- /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 0000000..3344881
--- /dev/null
+++ b/superset-frontend/images/icons/close.svg
@@ -0,0 +1,21 @@
+<!--
+ 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.
+-->
+<svg width="18" height="18" viewBox="0 0 18 18" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path fill-rule="evenodd" clip-rule="evenodd" d="M10.4099 9L16.7099 2.71C17.1021 2.31788 17.1021 1.68212 16.7099 1.29C16.3178 0.89788 15.6821 0.89788 15.2899 1.29L8.99994 7.59L2.70994 1.29C2.31782 0.89788 1.68206 0.89788 1.28994 1.29C0.897817 1.68212 0.897817 2.31788 1.28994 2.71L7.58994 9L1.28994 15.29C1.10063 15.4778 0.994141 15.7334 0.994141 16C0.994141 16.2666 1.10063 16.5222 1.28994 16.71C1.47771 16.8993 1.7333 17.0058 1.99994 17.0058C2.26658 17.0058 2.52217 16.8993 2.70994 16.71L8. [...]
+</svg>
diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js b/superset-frontend/spec/javascripts/chart/chartActions_spec.js
index a525885..1371074 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 0ecd7ab..f72ca40 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 dd2dd6f..db154fb 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 <Alert bsStyle="warning">Query was stopped</Alert>;
} else if (query.state === 'failed') {
return (
- <ErrorMessageWithStackTrace
- error={query.errors?.[0]}
- message={query.errorMessage || undefined}
- link={query.link}
- />
+ <div className="result-set-error-message">
+ <ErrorMessageWithStackTrace
+ error={query?.errors?.[0]}
+ message={query.errorMessage || undefined}
+ link={query.link}
+ source="sqllab"
+ />
+ </div>
);
} 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 7b3fb9c..b09884d 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 9562304..ff5a570 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 (
<ErrorMessageWithStackTrace
- error={queryResponse?.errors?.[0]}
+ error={error}
message={chartAlert || queryResponse?.message}
link={queryResponse ? queryResponse.link : null}
+ source={dashboardId ? 'dashboard' : 'explore'}
stackTrace={chartStackTrace}
/>
);
diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js
index 67f83b5..1be2626 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 d3e2236..c50c007 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 5f3f679..04befca 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 <ErrorMessageComponent error={error} />;
+ return <ErrorMessageComponent error={error} source={source} />;
}
}
diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/components/ErrorMessage/IssueCode.tsx
similarity index 65%
copy from superset-frontend/src/setup/setupErrorMessages.ts
copy to superset-frontend/src/components/ErrorMessage/IssueCode.tsx
index 289b3cb..3754341 100644
--- a/superset-frontend/src/setup/setupErrorMessages.ts
+++ b/superset-frontend/src/components/ErrorMessage/IssueCode.tsx
@@ -16,9 +16,24 @@
* specific language governing permissions and limitations
* under the License.
*/
-import setupErrorMessagesExtra from './setupErrorMessagesExtra';
+import React from 'react';
-export default function setupErrorMessages() {
- // TODO: Register error messages to the ErrorMessageComponentRegistry once implemented
- setupErrorMessagesExtra();
+interface IssueCodeProps {
+ code: number;
+ message: string;
+}
+
+export default function IssueCode({ code, message }: IssueCodeProps) {
+ return (
+ <>
+ {message}{' '}
+ <a
+ href={`https://superset.apache.org/issue_code_reference.html#issue-${code}`}
+ rel="noopener noreferrer"
+ target="_blank"
+ >
+ <i className="fa fa-external-link" />
+ </a>
+ </>
+ );
}
diff --git a/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/TimeoutErrorMessage.tsx
new file mode 100644
index 0000000..2b7ff01
--- /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<TimeoutErrorExtra>) {
+ 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 = (
+ <>
+ <p>
+ {t('This may be triggered by:')}
+ <br />
+ {extra.issue_codes
+ .map<React.ReactNode>(issueCode => <IssueCode {...issueCode} />)
+ .reduce((prev, curr) => [prev, <br />, curr])}
+ </p>
+ {isVisualization && extra.owners && (
+ <>
+ <br />
+ <p>
+ {tn(
+ 'Please reach out to the Chart Owner for assistance.',
+ 'Please reach out to the Chart Owners for assistance.',
+ extra.owners.length,
+ )}
+ </p>
+ <p>
+ {tn(
+ 'Chart Owner: %s',
+ 'Chart Owners: %s',
+ extra.owners.length,
+ extra.owners.join(', '),
+ )}
+ </p>
+ </>
+ )}
+ </>
+ );
+
+ const copyText = `${title}
+${t('This may be triggered by:')}
+${extra.issue_codes.map(issueCode => issueCode.message).join('\n')}`;
+
+ return (
+ <ErrorAlert>
+ <div className="top-row">
+ <LeftSideContent>
+ <Icon
+ className="icon"
+ name="error"
+ color={supersetTheme.colors.error.base}
+ />
+ <strong>{t('Timeout Error')}</strong>
+ </LeftSideContent>
+ {!isExpandable && (
+ <a href="#" className="link" onClick={() => setIsModalOpen(true)}>
+ {t('See More')}
+ </a>
+ )}
+ </div>
+ {isExpandable ? (
+ <div className="error-body">
+ <p>{title}</p>
+ {!isMessageExpanded && (
+ <a
+ href="#"
+ className="link"
+ onClick={() => setIsMessageExpanded(true)}
+ >
+ {t('See More')}
+ </a>
+ )}
+ {isMessageExpanded && (
+ <>
+ <br />
+ {message}
+ <a
+ href="#"
+ className="link"
+ onClick={() => setIsMessageExpanded(false)}
+ >
+ {t('See Less')}
+ </a>
+ </>
+ )}
+ </div>
+ ) : (
+ <ErrorModal show={isModalOpen} onHide={() => setIsModalOpen(false)}>
+ <Modal.Header className="header">
+ <LeftSideContent>
+ <Icon
+ className="icon"
+ name="error"
+ color={supersetTheme.colors.error.base}
+ />
+ <div className="title">{t('Timeout Error')}</div>
+ </LeftSideContent>
+ <span
+ role="button"
+ tabIndex={0}
+ onClick={() => setIsModalOpen(false)}
+ >
+ <Icon name="close" />
+ </span>
+ </Modal.Header>
+ <Modal.Body>
+ <p>{title}</p>
+ <br />
+ {message}
+ </Modal.Body>
+ <Modal.Footer>
+ <CopyToClipboard
+ text={copyText}
+ shouldShowText={false}
+ wrapped={false}
+ copyNode={<Button onClick={noOp}>{t('Copy Message')}</Button>}
+ />
+ <Button bsStyle="primary" onClick={() => setIsModalOpen(false)}>
+ {t('Close')}
+ </Button>
+ </Modal.Footer>
+ </ErrorModal>
+ )}
+ </ErrorAlert>
+ );
+}
+
+export default TimeoutErrorMessage;
diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts
index 5834c2c..e5b1b68 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> = T[keyof T];
@@ -46,15 +49,20 @@ export type ErrorType = ValueOf<typeof ErrorTypeEnum>;
// 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<ExtraType = Record<string, any> | null> = {
error_type: ErrorType;
- extra: Record<string, any> | null;
+ extra: ExtraType;
level: ErrorLevel;
message: string;
};
-export type ErrorMessageComponentProps = {
- error: SupersetError;
+export type ErrorMessageComponentProps<
+ ExtraType = Record<string, any> | null
+> = {
+ error: SupersetError<ExtraType>;
+ source?: ErrorSource;
};
export type ErrorMessageComponent = React.ComponentType<
diff --git a/superset-frontend/src/components/Icon.tsx b/superset-frontend/src/components/Icon.tsx
index e7dfe8d..1c8541a 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<SVGProps<SVGSVGElement>>
+> = {
'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<SVGSVGElement> {
- name: Icon;
+ name: IconName;
}
-const Icon = ({ name, ...rest }: IconProps) => {
+const Icon = ({ name, color = '#666666', ...rest }: IconProps) => {
const Component = iconsRegistry[name];
- return <Component {...rest} />;
+ return <Component color={color} {...rest} />;
};
-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 4a443f1..96ea557 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}
/>
</div>
</div>
diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js
index 3d4e38a..4f77b07 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 6b41b8a..e1eb01c 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 409ce5e..36975a0 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 289b3cb..269e7c5 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 2681827..e43def8 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 421aa01..369e5c5 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 54eb0ed..f2fda00 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 c849d67..3dca1de 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 b8f1d93..30f56ca 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 c464d78..c6d895c 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 0e17f57..7aeae79 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 bce09963..42c2f97 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 2e9ab4b..2d18751 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