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 e023d0bd896a6a448fd8eebe6a1bddbeda4c8053 Author: Beto Dealmeida <[email protected]> AuthorDate: Tue Sep 5 12:13:18 2023 -0700 wIP --- .../src/components/Datasource/DatasourceModal.tsx | 12 +++- .../ErrorMessage/MarshmallowErrorMessage.tsx | 76 ++++++++++++++++++++++ .../src/components/ErrorMessage/types.ts | 3 +- superset-frontend/src/setup/setupErrorMessages.ts | 5 ++ superset/datasets/api.py | 1 - superset/datasets/schemas.py | 8 +++ superset/errors.py | 3 + superset/exceptions.py | 13 ++++ 8 files changed, 116 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx b/superset-frontend/src/components/Datasource/DatasourceModal.tsx index f9c40c47ba..8dd6a23d56 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,17 @@ 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'), okButtonProps: { danger: true, className: 'btn-danger' }, + content: ( + <ErrorMessageWithStackTrace + subtitle="Subtitle" + 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..f6c0562866 --- /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 { t } from '@superset-ui/core'; + +import { ErrorMessageComponentProps } from './types'; +import IssueCode from './IssueCode'; +import ErrorAlert from './ErrorAlert'; + +interface DatabaseErrorExtra { + messages: object; + issue_codes: { + code: number; + message: string; + }[]; +} + +function MarshmallowErrorMessage({ + error, + source = 'crud', + subtitle, +}: ErrorMessageComponentProps<DatabaseErrorExtra>) { + const { extra, level, message } = error; + + const body = extra && ( + <> + <p> + {t('This may be triggered by:')} + <br /> + {extra.issue_codes + .map<React.ReactNode>(issueCode => ( + <IssueCode {...issueCode} key={issueCode.code} /> + )) + .reduce((prev, curr) => [prev, <br />, curr])} + </p> + </> + ); + + const copyText = extra?.issue_codes + ? t('%(message)s\nThis may be triggered by: \n%(issues)s', { + message, + issues: extra.issue_codes + .map(issueCode => issueCode.message) + .join('\n'), + }) + : message; + + return ( + <ErrorAlert + title={t('Error validating request')} + subtitle={subtitle} + level={level} + source={source} + copyText={copyText} + body={body} + /> + ); +} + +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/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..b62edf42b6 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..0696dabce1 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -278,3 +278,16 @@ class QueryNotFoundException(SupersetException): class ColumnNotFoundException(SupersetException): status = 404 + + +class SupersetMarshmallowValidationError(SupersetErrorException): + status = 422 + + def __init__(self, exc: ValidationError): + error = SupersetError( + message="An error happened when validating the request", + error_type=SupersetErrorType.MARSHMALLOW_ERROR, + level=ErrorLevel.ERROR, + extra={"messages": exc.messages}, + ) + super().__init__(error)
