This is an automated email from the ASF dual-hosted git repository.
maximebeauchemin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new e749efcb97 fix: refactor view error handling into a separate module
(#29330)
e749efcb97 is described below
commit e749efcb970a41d8e6282a7cb0a92e4f68453da2
Author: Maxime Beauchemin <[email protected]>
AuthorDate: Tue Jul 9 10:16:40 2024 -0700
fix: refactor view error handling into a separate module (#29330)
---
superset/databases/api.py | 4 +-
superset/initialization/__init__.py | 3 +
superset/utils/core.py | 2 +-
superset/views/api.py | 3 +-
superset/views/base.py | 192 +------------------------------
superset/views/base_api.py | 2 +-
superset/views/core.py | 12 +-
superset/views/datasource/views.py | 2 +-
superset/views/error_handling.py | 222 ++++++++++++++++++++++++++++++++++++
9 files changed, 236 insertions(+), 206 deletions(-)
diff --git a/superset/databases/api.py b/superset/databases/api.py
index 3a672eb766..af5ce255ae 100644
--- a/superset/databases/api.py
+++ b/superset/databases/api.py
@@ -123,13 +123,13 @@ from superset.utils import json
from superset.utils.core import error_msg_from_exception,
parse_js_uri_path_item
from superset.utils.oauth2 import decode_oauth2_state
from superset.utils.ssh_tunnel import mask_password_info
-from superset.views.base import json_errors_response
from superset.views.base_api import (
BaseSupersetModelRestApi,
requires_form_data,
requires_json,
statsd_metrics,
)
+from superset.views.error_handling import json_error_response
logger = logging.getLogger(__name__)
@@ -458,7 +458,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
except DatabaseConnectionFailedError as ex:
return self.response_422(message=str(ex))
except SupersetErrorsException as ex:
- return json_errors_response(errors=ex.errors, status=ex.status)
+ return json_error_response(ex.errors, status=ex.status)
except DatabaseCreateFailedError as ex:
logger.error(
"Error creating model %s: %s",
diff --git a/superset/initialization/__init__.py
b/superset/initialization/__init__.py
index f074eaf293..7c35fd17a7 100644
--- a/superset/initialization/__init__.py
+++ b/superset/initialization/__init__.py
@@ -174,6 +174,7 @@ class SupersetAppInitializer: # pylint:
disable=too-many-public-methods
from superset.views.database.views import DatabaseView
from superset.views.datasource.views import DatasetEditor, Datasource
from superset.views.dynamic_plugins import DynamicPluginsView
+ from superset.views.error_handling import set_app_error_handlers
from superset.views.explore import ExplorePermalinkView, ExploreView
from superset.views.key_value import KV
from superset.views.log.api import LogRestApi
@@ -188,6 +189,8 @@ class SupersetAppInitializer: # pylint:
disable=too-many-public-methods
from superset.views.tags import TagModelView, TagView
from superset.views.users.api import CurrentUserRestApi, UserRestApi
+ set_app_error_handlers(self.superset_app)
+
#
# Setup API views
#
diff --git a/superset/utils/core.py b/superset/utils/core.py
index d01dd517ef..b2f09aac2d 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -433,7 +433,7 @@ def error_msg_from_exception(ex: Exception) -> str:
msg = ex.message.get("message") # type: ignore
elif ex.message:
msg = ex.message
- return msg or str(ex)
+ return str(msg) or str(ex)
def markdown(raw: str, markup_wrap: bool | None = False) -> str:
diff --git a/superset/views/api.py b/superset/views/api.py
index 8ef59cb482..736c0b5548 100644
--- a/superset/views/api.py
+++ b/superset/views/api.py
@@ -34,7 +34,8 @@ from superset.models.slice import Slice
from superset.superset_typing import FlaskResponse
from superset.utils import json
from superset.utils.date_parser import get_since_until
-from superset.views.base import api, BaseSupersetView, handle_api_exception
+from superset.views.base import api, BaseSupersetView
+from superset.views.error_handling import handle_api_exception
if TYPE_CHECKING:
from superset.common.query_context_factory import QueryContextFactory
diff --git a/superset/views/base.py b/superset/views/base.py
index b836c5076c..f809d616c8 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -16,14 +16,12 @@
# under the License.
from __future__ import annotations
-import dataclasses
import functools
import logging
import os
import traceback
from datetime import datetime
-from importlib.resources import files
-from typing import Any, Callable, cast
+from typing import Any, Callable
import yaml
from babel import Locale
@@ -33,9 +31,7 @@ from flask import (
g,
get_flashed_messages,
redirect,
- request,
Response,
- send_file,
session,
)
from flask_appbuilder import BaseView, expose, Model, ModelView
@@ -52,11 +48,8 @@ from flask_appbuilder.security.sqla.models import User
from flask_appbuilder.widgets import ListWidget
from flask_babel import get_locale, gettext as __
from flask_jwt_extended.exceptions import NoAuthorizationError
-from flask_wtf.csrf import CSRFError
from flask_wtf.form import FlaskForm
-from sqlalchemy import exc
from sqlalchemy.orm import Query
-from werkzeug.exceptions import HTTPException
from wtforms.fields.core import Field, UnboundField
from superset import (
@@ -68,17 +61,9 @@ from superset import (
is_feature_enabled,
security_manager,
)
-from superset.commands.exceptions import CommandException, CommandInvalidError
from superset.connectors.sqla import models
from superset.db_engine_specs import get_available_engine_specs
from superset.db_engine_specs.gsheets import GSheetsEngineSpec
-from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
-from superset.exceptions import (
- SupersetErrorException,
- SupersetErrorsException,
- SupersetException,
- SupersetSecurityException,
-)
from superset.extensions import cache_manager
from superset.models.helpers import ImportExportMixin
from superset.reports.models import ReportRecipientType
@@ -86,6 +71,7 @@ from superset.superset_typing import FlaskResponse
from superset.translations.utils import get_language_pack
from superset.utils import core as utils, json
from superset.utils.filters import get_dataset_access_filters
+from superset.views.error_handling import json_error_response
from .utils import bootstrap_user_data
@@ -146,35 +132,6 @@ def get_error_msg() -> str:
return error_msg
-def json_error_response(
- msg: str | None = None,
- status: int = 500,
- payload: dict[str, Any] | None = None,
-) -> FlaskResponse:
- payload = payload or {"error": f"{msg}"}
-
- return Response(
- json.dumps(payload, default=json.json_iso_dttm_ser, ignore_nan=True),
- status=status,
- mimetype="application/json",
- )
-
-
-def json_errors_response(
- errors: list[SupersetError],
- status: int = 500,
- payload: dict[str, Any] | None = None,
-) -> FlaskResponse:
- payload = payload or {}
-
- payload["errors"] = [dataclasses.asdict(error) for error in errors]
- return Response(
- json.dumps(payload, default=json.json_iso_dttm_ser, ignore_nan=True),
- status=status,
- mimetype="application/json; charset=utf-8",
- )
-
-
def json_success(json_msg: str, status: int = 200) -> FlaskResponse:
return Response(json_msg, status=status, mimetype="application/json")
@@ -243,50 +200,6 @@ def api(f: Callable[..., FlaskResponse]) -> Callable[...,
FlaskResponse]:
return functools.update_wrapper(wraps, f)
-def handle_api_exception(
- f: Callable[..., FlaskResponse],
-) -> Callable[..., FlaskResponse]:
- """
- A decorator to catch superset exceptions. Use it after the @api decorator
above
- so superset exception handler is triggered before the handler for generic
- exceptions.
- """
-
- def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) ->
FlaskResponse:
- try:
- return f(self, *args, **kwargs)
- except SupersetSecurityException as ex:
- logger.warning("SupersetSecurityException", exc_info=True)
- return json_errors_response(
- errors=[ex.error], status=ex.status, payload=ex.payload
- )
- except SupersetErrorsException as ex:
- logger.warning(ex, exc_info=True)
- return json_errors_response(errors=ex.errors, status=ex.status)
- except SupersetErrorException as ex:
- logger.warning("SupersetErrorException", exc_info=True)
- return json_errors_response(errors=[ex.error], status=ex.status)
- except SupersetException as ex:
- if ex.status >= 500:
- logger.exception(ex)
- return json_error_response(
- utils.error_msg_from_exception(ex), status=ex.status
- )
- except HTTPException as ex:
- logger.exception(ex)
- return json_error_response(
- utils.error_msg_from_exception(ex), status=cast(int, ex.code)
- )
- except (exc.IntegrityError, exc.DatabaseError, exc.DataError) as ex:
- logger.exception(ex)
- return json_error_response(utils.error_msg_from_exception(ex),
status=422)
- except Exception as ex: # pylint: disable=broad-except
- logger.exception(ex)
- return json_error_response(utils.error_msg_from_exception(ex))
-
- return functools.update_wrapper(wraps, f)
-
-
class BaseSupersetView(BaseView):
@staticmethod
def json_response(obj: Any, status: int = 200) -> FlaskResponse:
@@ -439,107 +352,6 @@ def common_bootstrap_payload() -> dict[str, Any]:
}
-def get_error_level_from_status_code( # pylint: disable=invalid-name
- 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("SupersetErrorException", exc_info=True)
- return json_errors_response(errors=[ex.error], status=ex.status)
-
-
-@superset_app.errorhandler(SupersetErrorsException)
-def show_superset_errors(ex: SupersetErrorsException) -> FlaskResponse:
- logger.warning("SupersetErrorsException", exc_info=True)
- return json_errors_response(errors=ex.errors, status=ex.status)
-
-
-# Redirect to login if the CSRF token is expired
-@superset_app.errorhandler(CSRFError)
-def refresh_csrf_token(ex: CSRFError) -> FlaskResponse:
- logger.warning("Refresh CSRF token error", exc_info=True)
-
- if request.is_json:
- return show_http_exception(ex)
-
- return redirect(appbuilder.get_url_for_login)
-
-
-@superset_app.errorhandler(HTTPException)
-def show_http_exception(ex: HTTPException) -> FlaskResponse:
- logger.warning("HTTPException", exc_info=True)
- if (
- "text/html" in request.accept_mimetypes
- and not config["DEBUG"]
- and ex.code in {404, 500}
- ):
- path = files("superset") / f"static/assets/{ex.code}.html"
- return send_file(path, max_age=0), ex.code
-
- return json_errors_response(
- errors=[
- SupersetError(
- message=utils.error_msg_from_exception(ex),
- error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
- level=ErrorLevel.ERROR,
- ),
- ],
- 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("CommandException", exc_info=True)
- if "text/html" in request.accept_mimetypes and not config["DEBUG"]:
- path = files("superset") / "static/assets/500.html"
- return send_file(path, max_age=0), 500
-
- 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.exception(ex)
- if "text/html" in request.accept_mimetypes and not config["DEBUG"]:
- path = files("superset") / "static/assets/500.html"
- return send_file(path, max_age=0), 500
-
- return json_errors_response(
- errors=[
- SupersetError(
- message=utils.error_msg_from_exception(ex),
- error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
- level=ErrorLevel.ERROR,
- ),
- ],
- )
-
-
@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 9436ce6390..5c71147517 100644
--- a/superset/views/base_api.py
+++ b/superset/views/base_api.py
@@ -42,7 +42,7 @@ from superset.sql_lab import Query as SqllabQuery
from superset.superset_typing import FlaskResponse
from superset.tags.models import Tag
from superset.utils.core import get_user_id, time_function
-from superset.views.base import handle_api_exception
+from superset.views.error_handling import handle_api_exception
logger = logging.getLogger(__name__)
get_related_schema = {
diff --git a/superset/views/core.py b/superset/views/core.py
index 75a04dedc7..56221b646a 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -23,7 +23,7 @@ from datetime import datetime
from typing import Any, Callable, cast
from urllib import parse
-from flask import abort, flash, g, redirect, render_template, request, Response
+from flask import abort, flash, g, redirect, request, Response
from flask_appbuilder import expose
from flask_appbuilder.security.decorators import (
has_access,
@@ -85,11 +85,10 @@ from superset.views.base import (
data_payload_response,
deprecated,
generate_download_headers,
- get_error_msg,
- handle_api_exception,
json_error_response,
json_success,
)
+from superset.views.error_handling import handle_api_exception
from superset.views.utils import (
bootstrap_user_data,
check_datasource_perms,
@@ -888,13 +887,6 @@ class Superset(BaseSupersetView):
datasource.raise_for_access()
return
json_success(json.dumps(sanitize_datasource_data(datasource.data)))
- @app.errorhandler(500)
- def show_traceback(self) -> FlaskResponse:
- return (
- render_template("superset/traceback.html",
error_msg=get_error_msg()),
- 500,
- )
-
@event_logger.log_this
@expose("/welcome/")
def welcome(self) -> FlaskResponse:
diff --git a/superset/views/datasource/views.py
b/superset/views/datasource/views.py
index 377579cf05..fd0dc2590a 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -44,7 +44,6 @@ from superset.views.base import (
api,
BaseSupersetView,
deprecated,
- handle_api_exception,
json_error_response,
)
from superset.views.datasource.schemas import (
@@ -55,6 +54,7 @@ from superset.views.datasource.schemas import (
SamplesRequestSchema,
)
from superset.views.datasource.utils import get_samples
+from superset.views.error_handling import handle_api_exception
from superset.views.utils import sanitize_datasource_data
diff --git a/superset/views/error_handling.py b/superset/views/error_handling.py
new file mode 100644
index 0000000000..b9d0a41086
--- /dev/null
+++ b/superset/views/error_handling.py
@@ -0,0 +1,222 @@
+# 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 __future__ import annotations
+
+import dataclasses
+import functools
+import logging
+import typing
+from importlib.resources import files
+from typing import Any, Callable, cast
+
+from flask import (
+ Flask,
+ redirect,
+ request,
+ Response,
+ send_file,
+)
+from flask_wtf.csrf import CSRFError
+from sqlalchemy import exc
+from werkzeug.exceptions import HTTPException
+
+from superset import appbuilder
+from superset.commands.exceptions import CommandException, CommandInvalidError
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import (
+ SupersetErrorException,
+ SupersetErrorsException,
+ SupersetException,
+ SupersetSecurityException,
+)
+from superset.superset_typing import FlaskResponse
+from superset.utils import core as utils, json
+
+if typing.TYPE_CHECKING:
+ from superset.views.base import BaseSupersetView
+
+
+logger = logging.getLogger(__name__)
+
+JSON_MIMETYPE = "application/json; charset=utf-8"
+
+
+def get_error_level_from_status(
+ status_code: int,
+) -> ErrorLevel:
+ if status_code < 400:
+ return ErrorLevel.INFO
+ if status_code < 500:
+ return ErrorLevel.WARNING
+ return ErrorLevel.ERROR
+
+
+def json_error_response(
+ error_details: str | SupersetError | list[SupersetError] | None = None,
+ status: int = 500,
+ payload: dict[str, Any] | None = None,
+) -> FlaskResponse:
+ payload = payload or {}
+
+ if isinstance(error_details, list):
+ payload["errors"] = [dataclasses.asdict(error) for error in
error_details]
+ elif isinstance(error_details, SupersetError):
+ payload["errors"] = [dataclasses.asdict(error_details)]
+ elif isinstance(error_details, str):
+ payload["error"] = error_details
+
+ return Response(
+ json.dumps(payload, default=json.json_iso_dttm_ser, ignore_nan=True),
+ status=status,
+ mimetype=JSON_MIMETYPE,
+ )
+
+
+def handle_api_exception(
+ f: Callable[..., FlaskResponse],
+) -> Callable[..., FlaskResponse]:
+ """
+ A decorator to catch superset exceptions. Use it after the @api decorator
above
+ so superset exception handler is triggered before the handler for generic
+ exceptions.
+ """
+
+ def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) ->
FlaskResponse:
+ try:
+ return f(self, *args, **kwargs)
+ except SupersetSecurityException as ex:
+ logger.warning("SupersetSecurityException", exc_info=True)
+ return json_error_response([ex.error], status=ex.status,
payload=ex.payload)
+ except SupersetErrorsException as ex:
+ logger.warning(ex, exc_info=True)
+ return json_error_response(ex.errors, status=ex.status)
+ except SupersetErrorException as ex:
+ logger.warning("SupersetErrorException", exc_info=True)
+ return json_error_response([ex.error], status=ex.status)
+ except SupersetException as ex:
+ if ex.status >= 500:
+ logger.exception(ex)
+ return json_error_response(
+ utils.error_msg_from_exception(ex), status=ex.status
+ )
+ except HTTPException as ex:
+ logger.exception(ex)
+ return json_error_response(
+ utils.error_msg_from_exception(ex), status=cast(int, ex.code)
+ )
+ except (exc.IntegrityError, exc.DatabaseError, exc.DataError) as ex:
+ logger.exception(ex)
+ return json_error_response(utils.error_msg_from_exception(ex),
status=422)
+ except Exception as ex: # pylint: disable=broad-except
+ logger.exception(ex)
+ return json_error_response(utils.error_msg_from_exception(ex))
+
+ return functools.update_wrapper(wraps, f)
+
+
+def set_app_error_handlers(app: Flask) -> None:
+ """
+ Set up error handlers for the Flask app
+ Refer to SIP-40 and SIP-41 for more details on the error handling strategy
+ """
+
+ @app.errorhandler(SupersetErrorException)
+ def show_superset_error(ex: SupersetErrorException) -> FlaskResponse:
+ logger.warning("SupersetErrorException", exc_info=True)
+ return json_error_response([ex.error], status=ex.status)
+
+ @app.errorhandler(SupersetErrorsException)
+ def show_superset_errors(ex: SupersetErrorsException) -> FlaskResponse:
+ logger.warning("SupersetErrorsException", exc_info=True)
+ return json_error_response(ex.errors, status=ex.status)
+
+ @app.errorhandler(CSRFError)
+ def refresh_csrf_token(ex: CSRFError) -> FlaskResponse:
+ """Redirect to login if the CSRF token is expired"""
+ logger.warning("Refresh CSRF token error", exc_info=True)
+
+ if request.is_json:
+ return show_http_exception(ex)
+
+ return redirect(appbuilder.get_url_for_login)
+
+ @app.errorhandler(HTTPException)
+ def show_http_exception(ex: HTTPException) -> FlaskResponse:
+ logger.warning("HTTPException", exc_info=True)
+ if (
+ "text/html" in request.accept_mimetypes
+ and not app.config["DEBUG"]
+ and ex.code in {404, 500}
+ ):
+ path = files("superset") / f"static/assets/{ex.code}.html"
+ return send_file(path, max_age=0), ex.code
+
+ return json_error_response(
+ [
+ SupersetError(
+ message=utils.error_msg_from_exception(ex),
+ error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
+ level=ErrorLevel.ERROR,
+ ),
+ ],
+ status=ex.code or 500,
+ )
+
+ @app.errorhandler(CommandException)
+ def show_command_errors(ex: CommandException) -> FlaskResponse:
+ """
+ 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
+ """
+ logger.warning("CommandException", exc_info=True)
+ if "text/html" in request.accept_mimetypes and not app.config["DEBUG"]:
+ path = files("superset") / "static/assets/500.html"
+ return send_file(path, max_age=0), 500
+
+ extra = ex.normalized_messages() if isinstance(ex,
CommandInvalidError) else {}
+ return json_error_response(
+ [
+ SupersetError(
+ message=ex.message,
+ error_type=SupersetErrorType.GENERIC_COMMAND_ERROR,
+ level=get_error_level_from_status(ex.status),
+ extra=extra,
+ ),
+ ],
+ status=ex.status,
+ )
+
+ @app.errorhandler(Exception)
+ @app.errorhandler(500)
+ def show_unexpected_exception(ex: Exception) -> FlaskResponse:
+ """Catch-all, to ensure all errors from the backend conform to
SIP-40"""
+ logger.warning("Exception", exc_info=True)
+ logger.exception(ex)
+ if "text/html" in request.accept_mimetypes and not app.config["DEBUG"]:
+ path = files("superset") / "static/assets/500.html"
+ return send_file(path, max_age=0), 500
+
+ return json_error_response(
+ [
+ SupersetError(
+ message=utils.error_msg_from_exception(ex),
+ error_type=SupersetErrorType.GENERIC_BACKEND_ERROR,
+ level=ErrorLevel.ERROR,
+ ),
+ ],
+ )
