You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/10/03 00:13:29 UTC
[superset] 01/05: feat: generic marshmallow error component
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 <ro...@dealmeida.net>
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)