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 e39de65ebfa6bdb0e41c4d5a4054a51f09f66217 Author: Beto Dealmeida <[email protected]> AuthorDate: Tue Sep 5 12:13:18 2023 -0700 wIP --- .../src/components/Datasource/DatasourceModal.tsx | 13 ++-- .../ErrorMessage/MarshmallowErrorMessage.tsx | 76 ++++++++++++++++++++++ .../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 | 8 +++ superset/errors.py | 3 + superset/exceptions.py | 19 +++++- 10 files changed, 165 insertions(+), 34 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index f9c40c47ba..fde88ee280 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'; @@ -202,11 +202,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..dc305d1ae2 --- /dev/null +++ b/superset-frontend/src/components/ErrorMessage/MarshmallowErrorMessage.tsx @@ -0,0 +1,76 @@ +/** + * 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 } from '@superset-ui/core'; + +import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme'; +import Collapse from 'src/components/Collapse'; +import { ErrorMessageComponentProps } from './types'; + +interface MarshmallowErrorExtra { + messages: object; + issue_codes: { + code: number; + message: string; + }[]; +} + +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; + } +`; + +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} + + <Collapse ghost css={collapseStyle}> + <Collapse.Panel header="Details" key="details" css={collapseStyle}> + <JSONTree + data={extra.messages} + shouldExpandNode={() => true} + hideRoot + theme={jsonTreeTheme} + /> + </Collapse.Panel> + </Collapse> + </div> + ); +} + +export default MarshmallowErrorMessage; 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 9bbb6acd21..d68e1523bf 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -330,7 +330,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 66d80e1842..f47ca2181b 100644 --- a/superset/datasets/schemas.py +++ b/superset/datasets/schemas.py @@ -18,12 +18,14 @@ import json import re from typing import Any +import yaml from flask_babel import lazy_gettext as _ from marshmallow import fields, pre_load, Schema, ValidationError 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"}} @@ -123,6 +125,12 @@ 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, exc: ValidationError, data: dict[str, Any], **kwargs: Any): + """ + Return SIP-40 error. + """ + raise SupersetMarshmallowValidationError(exc) + class DatasetDuplicateSchema(Schema): base_model_id = fields.Integer(required=True) diff --git a/superset/errors.py b/superset/errors.py index 167ec2d3b4..b0d1d6faff 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..539f6193aa 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. from collections import defaultdict -from typing import Any, Optional +from typing import Any, Iterator, Optional, Tuple from flask_babel import gettext as _ from marshmallow import ValidationError @@ -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): + error = SupersetError( + message=_("The schema of the submitted payload is invalid."), + error_type=SupersetErrorType.MARSHMALLOW_ERROR, + level=ErrorLevel.ERROR, + extra={"messages": exc.messages}, + ) + super().__init__(error)
