diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx index 8b8e0c8e73cb1..cf259ab701f3c 100644 --- a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx +++ b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx @@ -113,3 +113,21 @@ The host might be down, and cannot be reached on the provided port. The host provided when adding a new database doesn't seem to be up. Additionally, it cannot be reached on the provided port. Please check that there are no firewall rules preventing access to the host. + +## Issue 1010 + +``` +Superset encountered an error while running a command. +``` + +Something unexpected happened, and Superset encountered an error while +running a command. Please reach out to your administrator. + +## Issue 1011 + +``` +Superset encountered an unexpected error. +``` + +Someething unexpected happened in the Superset backend. Please reach out +to your administrator. diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index 30cb2658beed2..2ffc655580224 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -28,6 +28,8 @@ export const ErrorTypeEnum = { GENERIC_DB_ENGINE_ERROR: 'GENERIC_DB_ENGINE_ERROR', COLUMN_DOES_NOT_EXIST_ERROR: 'COLUMN_DOES_NOT_EXIST_ERROR', TABLE_DOES_NOT_EXIST_ERROR: 'TABLE_DOES_NOT_EXIST_ERROR', + TEST_CONNECTION_PORT_CLOSED_ERROR: 'TEST_CONNECTION_PORT_CLOSED_ERROR', + TEST_CONNECTION_HOST_DOWN_ERROR: 'TEST_CONNECTION_HOST_DOWN_ERROR', // Viz errors VIZ_GET_DF_ERROR: 'VIZ_GET_DF_ERROR', @@ -48,8 +50,10 @@ export const ErrorTypeEnum = { MISSING_TEMPLATE_PARAMS_ERROR: 'MISSING_TEMPLATE_PARAMS_ERROR', TEST_CONNECTION_INVALID_HOSTNAME_ERROR: 'TEST_CONNECTION_INVALID_HOSTNAME_ERROR', - TEST_CONNECTION_PORT_CLOSED_ERROR: 'TEST_CONNECTION_PORT_CLOSED_ERROR', - TEST_CONNECTION_HOST_DOWN_ERROR: 'TEST_CONNECTION_HOST_DOWN_ERROR', + + // Generic errors + GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR', + GENERIC_BACKEND_ERROR: 'GENERIC_BACKEND_ERROR', } as const; type ValueOf = T[keyof T]; diff --git a/superset/databases/api.py b/superset/databases/api.py index ba43f3a09504f..970f61accf0c8 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -41,7 +41,6 @@ DatabaseImportError, DatabaseInvalidError, DatabaseNotFoundError, - DatabaseTestConnectionFailedError, DatabaseUpdateFailedError, ) from superset.databases.commands.export import ExportDatabasesCommand @@ -64,7 +63,6 @@ TableMetadataResponseSchema, ) from superset.databases.utils import get_table_metadata -from superset.exceptions import SupersetErrorException from superset.extensions import security_manager from superset.models.core import Database from superset.typing import FlaskResponse @@ -558,7 +556,6 @@ def select_star( @expose("/test_connection", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" @@ -604,13 +601,8 @@ def test_connection( # pylint: disable=too-many-return-statements # This validates custom Schema with custom validations except ValidationError as error: return self.response_400(message=error.messages) - try: - TestConnectionDatabaseCommand(g.user, item).run() - return self.response(200, message="OK") - except DatabaseTestConnectionFailedError as ex: - return self.response_422(message=str(ex)) - except SupersetErrorException as ex: - return self.response(ex.status, message=ex.error.message) + TestConnectionDatabaseCommand(g.user, item).run() + return self.response(200, message="OK") @expose("//related_objects/", methods=["GET"]) @protect() diff --git a/superset/databases/commands/exceptions.py b/superset/databases/commands/exceptions.py index a191ac96faa84..d2105ca962bfd 100644 --- a/superset/databases/commands/exceptions.py +++ b/superset/databases/commands/exceptions.py @@ -118,6 +118,7 @@ class DatabaseDeleteFailedReportsExistError(DatabaseDeleteFailedError): class DatabaseTestConnectionFailedError(CommandException): + status = 422 message = _("Connection failed, please check your connection settings") diff --git a/superset/errors.py b/superset/errors.py index 248d75d27b23b..13cd419df960f 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -39,6 +39,8 @@ class SupersetErrorType(str, Enum): GENERIC_DB_ENGINE_ERROR = "GENERIC_DB_ENGINE_ERROR" COLUMN_DOES_NOT_EXIST_ERROR = "COLUMN_DOES_NOT_EXIST_ERROR" TABLE_DOES_NOT_EXIST_ERROR = "TABLE_DOES_NOT_EXIST_ERROR" + TEST_CONNECTION_PORT_CLOSED_ERROR = "TEST_CONNECTION_PORT_CLOSED_ERROR" + TEST_CONNECTION_HOST_DOWN_ERROR = "TEST_CONNECTION_HOST_DOWN_ERROR" # Viz errors VIZ_GET_DF_ERROR = "VIZ_GET_DF_ERROR" @@ -57,8 +59,10 @@ class SupersetErrorType(str, Enum): # Sql Lab errors MISSING_TEMPLATE_PARAMS_ERROR = "MISSING_TEMPLATE_PARAMS_ERROR" TEST_CONNECTION_INVALID_HOSTNAME_ERROR = "TEST_CONNECTION_INVALID_HOSTNAME_ERROR" - TEST_CONNECTION_PORT_CLOSED_ERROR = "TEST_CONNECTION_PORT_CLOSED_ERROR" - TEST_CONNECTION_HOST_DOWN_ERROR = "TEST_CONNECTION_HOST_DOWN_ERROR" + + # Generic errors + GENERIC_COMMAND_ERROR = "GENERIC_COMMAND_ERROR" + GENERIC_BACKEND_ERROR = "GENERIC_BACKEND_ERROR" ERROR_TYPES_TO_ISSUE_CODES_MAPPING = { @@ -135,6 +139,20 @@ class SupersetErrorType(str, Enum): ), }, ], + SupersetErrorType.GENERIC_COMMAND_ERROR: [ + { + "code": 1010, + "message": _( + "Issue 1010 - Superset encountered an error while running a command." + ), + }, + ], + SupersetErrorType.GENERIC_BACKEND_ERROR: [ + { + "code": 1011, + "message": _("Issue 1011 - Superset encountered an unexpected error."), + }, + ], } diff --git a/superset/exceptions.py b/superset/exceptions.py index 1d5ff2ced13d7..42e089ba45ac5 100644 --- a/superset/exceptions.py +++ b/superset/exceptions.py @@ -54,6 +54,14 @@ def __init__( ) +class SupersetErrorsException(SupersetException): + """Exceptions with multiple SupersetErrorType associated with them""" + + def __init__(self, errors: List[SupersetError]) -> None: + super().__init__(str(errors)) + self.errors = errors + + class SupersetTimeoutException(SupersetErrorException): status = 408 @@ -97,13 +105,9 @@ def __init__( self.payload = payload -class SupersetVizException(SupersetException): +class SupersetVizException(SupersetErrorsException): status = 400 - def __init__(self, errors: List[SupersetError]) -> None: - super().__init__(str(errors)) - self.errors = errors - class NoDataException(SupersetException): status = 400 diff --git a/superset/schemas.py b/superset/schemas.py new file mode 100644 index 0000000000000..52fb95f4f9151 --- /dev/null +++ b/superset/schemas.py @@ -0,0 +1,49 @@ +# 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. + +from superset.errors import SupersetErrorType + +error_payload_content = { + "application/json": { + "schema": { + "type": "object", + "properties": { + # SIP-40 error payload + "errors": { + "type": "array", + "items": { + "type": "object", + "properties": { + "message": {"type": "string"}, + "error_type": { + "type": "string", + "enum": [enum.value for enum in SupersetErrorType], + }, + "level": { + "type": "string", + "enum": ["info", "warning", "error"], + }, + "extra": {"type": "object"}, + }, + }, + }, + # Old-style message payload + "message": {"type": "string"}, + }, + }, + }, +} diff --git a/superset/views/base.py b/superset/views/base.py index b5d7138050ed5..46f0d6fc6b7b0 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -46,11 +46,13 @@ get_feature_flags, security_manager, ) +from superset.commands.exceptions import CommandException, CommandInvalidError from superset.connectors.sqla import models from superset.datasets.commands.exceptions import get_dataset_exist_error_msg from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import ( SupersetErrorException, + SupersetErrorsException, SupersetException, SupersetSecurityException, ) @@ -132,7 +134,7 @@ def json_errors_response( return Response( json.dumps(payload, default=utils.json_iso_dttm_ser, ignore_nan=True), status=status, - mimetype="application/json", + mimetype="application/json; charset=utf-8", ) @@ -330,6 +332,81 @@ def common_bootstrap_payload() -> Dict[str, Any]: } +# pylint: disable=invalid-name +def get_error_level_from_status_code(status: int) -> ErrorLevel: + if status < 400: + return ErrorLevel.INFO + if status < 500: + return ErrorLevel.WARNING + return ErrorLevel.ERROR + + +# SIP-40 compatible error responses; make sure APIs raise +# SupersetErrorException or SupersetErrorsException +@superset_app.errorhandler(SupersetErrorException) +def show_superset_error(ex: SupersetErrorException) -> FlaskResponse: + logger.warning(ex) + return json_errors_response(errors=[ex.error], status=ex.status) + + +@superset_app.errorhandler(SupersetErrorsException) +def show_superset_errors(ex: SupersetErrorsException) -> FlaskResponse: + logger.warning(ex) + return json_errors_response(errors=ex.errors, status=ex.status) + + +@superset_app.errorhandler(HTTPException) +def show_http_exception(ex: HTTPException) -> FlaskResponse: + logger.warning(ex) + return json_errors_response( + errors=[ + SupersetError( + message=utils.error_msg_from_exception(ex), + error_type=SupersetErrorType.GENERIC_BACKEND_ERROR, + level=ErrorLevel.ERROR, + extra={}, + ), + ], + status=ex.code or 500, + ) + + +# Temporary handler for CommandException; if an API raises a +# CommandException it should be fixed to map it to SupersetErrorException +# or SupersetErrorsException, with a specific status code and error type +@superset_app.errorhandler(CommandException) +def show_command_errors(ex: CommandException) -> FlaskResponse: + logger.warning(ex) + extra = ex.normalized_messages() if isinstance(ex, CommandInvalidError) else {} + return json_errors_response( + errors=[ + SupersetError( + message=ex.message, + error_type=SupersetErrorType.GENERIC_COMMAND_ERROR, + level=get_error_level_from_status_code(ex.status), + extra=extra, + ), + ], + status=ex.status, + ) + + +# Catch-all, to ensure all errors from the backend conform to SIP-40 +@superset_app.errorhandler(Exception) +def show_unexpected_exception(ex: Exception) -> FlaskResponse: + logger.warning(ex) + return json_errors_response( + errors=[ + SupersetError( + message=utils.error_msg_from_exception(ex), + error_type=SupersetErrorType.GENERIC_BACKEND_ERROR, + level=ErrorLevel.ERROR, + extra={}, + ), + ], + ) + + @superset_app.context_processor def get_common_bootstrap_data() -> Dict[str, Any]: def serialize_bootstrap_data() -> str: diff --git a/superset/views/base_api.py b/superset/views/base_api.py index 2956058108d66..11632d67f2d5d 100644 --- a/superset/views/base_api.py +++ b/superset/views/base_api.py @@ -35,6 +35,7 @@ from superset.models.core import FavStar from superset.models.dashboard import Dashboard from superset.models.slice import Slice +from superset.schemas import error_payload_content from superset.sql_lab import Query as SqllabQuery from superset.stats_logger import BaseStatsLogger from superset.typing import FlaskResponse @@ -77,7 +78,12 @@ def statsd_metrics(f: Callable[..., Any]) -> Callable[..., Any]: """ def wraps(self: "BaseSupersetModelRestApi", *args: Any, **kwargs: Any) -> Response: - duration, response = time_function(f, self, *args, **kwargs) + try: + duration, response = time_function(f, self, *args, **kwargs) + except Exception as ex: + self.incr_stats("error", f.__name__) + raise ex + self.send_stats_metrics(response, f.__name__, duration) return response @@ -198,6 +204,18 @@ class BaseSupersetModelRestApi(ModelRestApi): list_columns: List[str] show_columns: List[str] + responses = { + "400": {"description": "Bad request", "content": error_payload_content}, + "401": {"description": "Unauthorized", "content": error_payload_content}, + "403": {"description": "Forbidden", "content": error_payload_content}, + "404": {"description": "Not found", "content": error_payload_content}, + "422": { + "description": "Could not process entity", + "content": error_payload_content, + }, + "500": {"description": "Fatal error", "content": error_payload_content}, + } + def __init__(self) -> None: # Setup statsd self.stats_logger = BaseStatsLogger() diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index 897edd0e1e7a6..174bc29f63976 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -820,7 +820,21 @@ def test_test_connection_failed(self): self.assertEqual(rv.headers["Content-Type"], "application/json; charset=utf-8") response = json.loads(rv.data.decode("utf-8")) expected_response = { - "message": "Could not load database driver: BaseEngineSpec", + "errors": [ + { + "message": "Could not load database driver: BaseEngineSpec", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "issue_codes": [ + { + "code": 1010, + "message": "Issue 1010 - Superset encountered an error while running a command.", + } + ] + }, + } + ] } self.assertEqual(response, expected_response) @@ -835,7 +849,21 @@ def test_test_connection_failed(self): self.assertEqual(rv.headers["Content-Type"], "application/json; charset=utf-8") response = json.loads(rv.data.decode("utf-8")) expected_response = { - "message": "Could not load database driver: MssqlEngineSpec", + "errors": [ + { + "message": "Could not load database driver: MssqlEngineSpec", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "issue_codes": [ + { + "code": 1010, + "message": "Issue 1010 - Superset encountered an error while running a command.", + } + ] + }, + } + ] } self.assertEqual(response, expected_response) @@ -888,7 +916,22 @@ def test_test_connection_failed_invalid_hostname(self, mock_is_hostname_valid): assert rv.headers["Content-Type"] == "application/json; charset=utf-8" response = json.loads(rv.data.decode("utf-8")) expected_response = { - "message": 'Unable to resolve hostname "invalidhostname".', + "errors": [ + { + "message": 'Unable to resolve hostname "invalidhostname".', + "error_type": "TEST_CONNECTION_INVALID_HOSTNAME_ERROR", + "level": "error", + "extra": { + "hostname": "invalidhostname", + "issue_codes": [ + { + "code": 1007, + "message": "Issue 1007 - The hostname provided can't be resolved.", + } + ], + }, + } + ] } assert response == expected_response @@ -919,7 +962,23 @@ def test_test_connection_failed_closed_port( assert rv.headers["Content-Type"] == "application/json; charset=utf-8" response = json.loads(rv.data.decode("utf-8")) expected_response = { - "message": "The host localhost is up, but the port 12345 is closed.", + "errors": [ + { + "message": "The host localhost is up, but the port 12345 is closed.", + "error_type": "TEST_CONNECTION_PORT_CLOSED_ERROR", + "level": "error", + "extra": { + "hostname": "localhost", + "port": 12345, + "issue_codes": [ + { + "code": 1008, + "message": "Issue 1008 - The port is closed.", + } + ], + }, + } + ] } assert response == expected_response @@ -950,7 +1009,23 @@ def test_test_connection_failed_host_down( assert rv.headers["Content-Type"] == "application/json; charset=utf-8" response = json.loads(rv.data.decode("utf-8")) expected_response = { - "message": "The host localhost might be down, ond can't be reached on port 12345.", + "errors": [ + { + "message": "The host localhost might be down, ond can't be reached on port 12345.", + "error_type": "TEST_CONNECTION_HOST_DOWN_ERROR", + "level": "error", + "extra": { + "hostname": "localhost", + "port": 12345, + "issue_codes": [ + { + "code": 1009, + "message": "Issue 1009 - The host might be down, and can't be reached on the provided port.", + } + ], + }, + } + ] } assert response == expected_response