This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch sc_71594 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 544a6be81262a6bedec015ce29aadfd7a92233d6 Author: Beto Dealmeida <[email protected]> AuthorDate: Tue Sep 5 12:13:18 2023 -0700 feat: generic marshmallow error component --- .../src/components/Datasource/DatasourceModal.tsx | 13 ++- .../ErrorMessage/MarshmallowErrorMessage.tsx | 109 +++++++++++++++++++++ .../src/components/ErrorMessage/types.ts | 3 +- .../src/components/FilterableTable/index.tsx | 30 +----- superset-frontend/src/hooks/useJsonTreeTheme.ts | 41 ++++++++ superset-frontend/src/setup/setupErrorMessages.ts | 5 + superset/datasets/api.py | 1 - superset/datasets/schemas.py | 12 +++ superset/errors.py | 3 + superset/exceptions.py | 17 ++++ 10 files changed, 201 insertions(+), 33 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index d4239136a7..fa88989f8a 100644 --- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx +++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx @@ -31,7 +31,7 @@ import { import Modal from 'src/components/Modal'; import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; -import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import withToasts from 'src/components/MessageToasts/withToasts'; import { useSelector } from 'react-redux'; @@ -203,11 +203,16 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({ }) .catch(response => { setIsSaving(false); - getClientErrorObject(response).then(({ error }) => { + response.json().then(errorJson => { modal.error({ - title: t('Error'), - content: error || t('An error has occurred'), + title: t('Error saving dataset'), okButtonProps: { danger: true, className: 'btn-danger' }, + content: ( + <ErrorMessageWithStackTrace + error={errorJson.errors[0]} + source="crud" + /> + ), }); }); }); diff --git a/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx b/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx new file mode 100644 index 0000000000..0718840121 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx @@ -0,0 +1,109 @@ +/** + * 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'; +import { JSONTree } from 'react-json-tree'; +import { css, styled, t } from '@superset-ui/core'; + +import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme'; +import Collapse from 'src/components/Collapse'; +import { ErrorMessageComponentProps } from './types'; + +interface MarshmallowErrorExtra { + messages: object; + payload: object; + issue_codes: { + code: number; + message: string; + }[]; +} + +const StyledUl = styled.ul` + padding-left: ${({ theme }) => theme.gridUnit * 5}px; + padding-top: ${({ theme }) => theme.gridUnit * 4}px; +`; + +const collapseStyle = theme => css` + .ant-collapse-arrow { + left: 0px !important; + } + .ant-collapse-header { + padding-left: ${theme.gridUnit * 4}px !important; + } + .ant-collapse-content-box { + padding: 0px !important; + } +`; + +const extractInvalidValues = (messages: object, payload: object): string[] => { + const invalidValues: string[] = []; + + const recursiveExtract = (messages: object, payload: object) => { + Object.keys(messages).forEach(key => { + const value = payload[key]; + const message = messages[key]; + + if (Array.isArray(message)) { + message.forEach(errorMessage => { + invalidValues.push(`${errorMessage}: ${value}`); + }); + } else { + recursiveExtract(message, value); + } + }); + }; + recursiveExtract(messages, payload); + return invalidValues; +}; + +export default function MarshmallowErrorMessage({ + error, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + source = 'crud', + subtitle, +}: ErrorMessageComponentProps<MarshmallowErrorExtra>) { + const jsonTreeTheme = useJsonTreeTheme(); + const { extra, message } = error; + + return ( + <div> + {subtitle && <h4>{subtitle}</h4>} + + {message} + + <StyledUl> + {extractInvalidValues(extra.messages, extra.payload).map( + (value, index) => ( + <li key={index}>{value}</li> + ), + )} + </StyledUl> + + <Collapse ghost css={collapseStyle}> + <Collapse.Panel header={t('Details')} key="details" css={collapseStyle}> + <JSONTree + data={extra.messages} + shouldExpandNode={() => true} + hideRoot + theme={jsonTreeTheme} + /> + </Collapse.Panel> + </Collapse> + </div> + ); +} diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index d3fe5bfdf7..7c4c3fe94a 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -79,6 +79,7 @@ export const ErrorTypeEnum = { // API errors INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR', INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR', + MARSHMALLOW_ERROR: 'MARSHMALLOW_ERROR', } as const; type ValueOf<T> = T[keyof T]; @@ -88,7 +89,7 @@ export type ErrorType = ValueOf<typeof ErrorTypeEnum>; // Keep in sync with superset/views/errors.py export type ErrorLevel = 'info' | 'warning' | 'error'; -export type ErrorSource = 'dashboard' | 'explore' | 'sqllab'; +export type ErrorSource = 'dashboard' | 'explore' | 'sqllab' | 'crud'; export type SupersetError<ExtraType = Record<string, any> | null> = { error_type: ErrorType; diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx index 91fc1f4477..8e7d54ef9f 100644 --- a/superset-frontend/src/components/FilterableTable/index.tsx +++ b/superset-frontend/src/components/FilterableTable/index.tsx @@ -27,6 +27,7 @@ import { useTheme, } from '@superset-ui/core'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; +import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme'; import Button from '../Button'; import CopyToClipboard from '../CopyToClipboard'; import ModalTrigger from '../ModalTrigger'; @@ -183,32 +184,7 @@ const FilterableTable = ({ return complexColumns[columnKey] ? truncated : content; }; - const theme = useTheme(); - const [jsonTreeTheme, setJsonTreeTheme] = useState<Record<string, string>>(); - - const getJsonTreeTheme = () => { - if (!jsonTreeTheme) { - setJsonTreeTheme({ - base00: theme.colors.grayscale.dark2, - base01: theme.colors.grayscale.dark1, - base02: theme.colors.grayscale.base, - base03: theme.colors.grayscale.light1, - base04: theme.colors.grayscale.light2, - base05: theme.colors.grayscale.light3, - base06: theme.colors.grayscale.light4, - base07: theme.colors.grayscale.light5, - base08: theme.colors.error.base, - base09: theme.colors.error.light1, - base0A: theme.colors.error.light2, - base0B: theme.colors.success.base, - base0C: theme.colors.primary.light1, - base0D: theme.colors.primary.base, - base0E: theme.colors.primary.dark1, - base0F: theme.colors.error.dark1, - }); - } - return jsonTreeTheme; - }; + const jsonTreeTheme = useJsonTreeTheme(); const getWidthsForColumns = () => { const PADDING = 50; // accounts for cell padding and width of sorting icon @@ -293,7 +269,7 @@ const FilterableTable = ({ modalBody={ <JSONTree data={jsonObject} - theme={getJsonTreeTheme()} + theme={jsonTreeTheme} valueRenderer={renderBigIntStrToNumber} /> } diff --git a/superset-frontend/src/hooks/useJsonTreeTheme.ts b/superset-frontend/src/hooks/useJsonTreeTheme.ts new file mode 100644 index 0000000000..a052e22a63 --- /dev/null +++ b/superset-frontend/src/hooks/useJsonTreeTheme.ts @@ -0,0 +1,41 @@ +/** + * 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 { useTheme } from '@superset-ui/core'; + +export const useJsonTreeTheme = () => { + const theme = useTheme(); + return { + base00: theme.colors.grayscale.dark2, + base01: theme.colors.grayscale.dark1, + base02: theme.colors.grayscale.base, + base03: theme.colors.grayscale.light1, + base04: theme.colors.grayscale.light2, + base05: theme.colors.grayscale.light3, + base06: theme.colors.grayscale.light4, + base07: theme.colors.grayscale.light5, + base08: theme.colors.error.base, + base09: theme.colors.error.light1, + base0A: theme.colors.error.light2, + base0B: theme.colors.success.base, + base0C: theme.colors.primary.light1, + base0D: theme.colors.primary.base, + base0E: theme.colors.primary.dark1, + base0F: theme.colors.error.dark1, + }; +}; diff --git a/superset-frontend/src/setup/setupErrorMessages.ts b/superset-frontend/src/setup/setupErrorMessages.ts index bebb22b20c..59842f190a 100644 --- a/superset-frontend/src/setup/setupErrorMessages.ts +++ b/superset-frontend/src/setup/setupErrorMessages.ts @@ -20,6 +20,7 @@ import getErrorMessageComponentRegistry from 'src/components/ErrorMessage/getErr import { ErrorTypeEnum } from 'src/components/ErrorMessage/types'; import TimeoutErrorMessage from 'src/components/ErrorMessage/TimeoutErrorMessage'; import DatabaseErrorMessage from 'src/components/ErrorMessage/DatabaseErrorMessage'; +import MarshmallowErrorMessage from 'src/components/ErrorMessage/MarshmallowErrorMessage'; import ParameterErrorMessage from 'src/components/ErrorMessage/ParameterErrorMessage'; import DatasetNotFoundErrorMessage from 'src/components/ErrorMessage/DatasetNotFoundErrorMessage'; @@ -144,5 +145,9 @@ export default function setupErrorMessages() { ErrorTypeEnum.FAILED_FETCHING_DATASOURCE_INFO_ERROR, DatasetNotFoundErrorMessage, ); + errorMessageComponentRegistry.registerValue( + ErrorTypeEnum.MARSHMALLOW_ERROR, + MarshmallowErrorMessage, + ); setupErrorMessagesExtra(); } diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 5c722cc0df..e1d7e5a09e 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -332,7 +332,6 @@ class DatasetRestApi(BaseSupersetModelRestApi): @expose("/<pk>", methods=("PUT",)) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.put", diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py index 0b137e96a7..84f0453fb4 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -24,6 +24,7 @@ from marshmallow.validate import Length from marshmallow_sqlalchemy import SQLAlchemyAutoSchema from superset.datasets.models import Dataset +from superset.exceptions import SupersetMarshmallowValidationError get_delete_ids_schema = {"type": "array", "items": {"type": "integer"}} get_export_ids_schema = {"type": "array", "items": {"type": "integer"}} @@ -125,6 +126,17 @@ class DatasetPutSchema(Schema): is_managed_externally = fields.Boolean(allow_none=True, dump_default=False) external_url = fields.String(allow_none=True) + def handle_error( + self, + error: ValidationError, + data: dict[str, Any], + **kwargs: Any, + ) -> None: + """ + Return SIP-40 error. + """ + raise SupersetMarshmallowValidationError(error, data) + class DatasetDuplicateSchema(Schema): base_model_id = fields.Integer(required=True) diff --git a/superset/errors.py b/superset/errors.py index a480ea3d28..87cdf77b99 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -90,6 +90,7 @@ class SupersetErrorType(StrEnum): # API errors INVALID_PAYLOAD_FORMAT_ERROR = "INVALID_PAYLOAD_FORMAT_ERROR" INVALID_PAYLOAD_SCHEMA_ERROR = "INVALID_PAYLOAD_SCHEMA_ERROR" + MARSHMALLOW_ERROR = "MARSHMALLOW_ERROR" # Report errors REPORT_NOTIFICATION_ERROR = "REPORT_NOTIFICATION_ERROR" @@ -144,6 +145,7 @@ ISSUE_CODES = { 1035: _("Failed to start remote query on a worker."), 1036: _("The database was deleted."), 1037: _("Custom SQL fields cannot contain sub-queries."), + 1040: _("The submitted payload failed validation."), } @@ -181,6 +183,7 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = { SupersetErrorType.ASYNC_WORKERS_ERROR: [1035], SupersetErrorType.DATABASE_NOT_FOUND_ERROR: [1011, 1036], SupersetErrorType.CONNECTION_DATABASE_TIMEOUT: [1001, 1009], + SupersetErrorType.MARSHMALLOW_ERROR: [1040], } diff --git a/superset/exceptions.py b/superset/exceptions.py index 5f1df73c86..3642a9279e 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -278,3 +278,20 @@ class QueryNotFoundException(SupersetException): class ColumnNotFoundException(SupersetException): status = 404 + + +class SupersetMarshmallowValidationError(SupersetErrorException): + """ + Exception to be raised for Marshmallow validation errors. + """ + + status = 422 + + def __init__(self, exc: ValidationError, payload: dict[str, Any]): + error = SupersetError( + message=_("The schema of the submitted payload is invalid."), + error_type=SupersetErrorType.MARSHMALLOW_ERROR, + level=ErrorLevel.ERROR, + extra={"messages": exc.messages, "payload": payload}, + ) + super().__init__(error)
