From cfb0c6bd744e2591e260f26e5c4b2e9432f7dd0d Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Fri, 31 Jul 2026 15:19:10 +0000 Subject: [PATCH 1/2] fix(errors): map uncaught SupersetException status/log-level correctly (SC-115609) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChartDataRestApi.data() isn't decorated with @handle_api_exception, so a correctly-classified QueryObjectValidationError (e.g. from a virtual dataset's Jinja template indexing into an empty filter_values() list) propagates uncaught to Flask's global catch-all, which always logs at ERROR and returns HTTP 500 regardless of the exception's real status. Add @app.errorhandler(SupersetException) mirroring the existing handle_api_exception pattern: log level and HTTP status now follow ex.status via get_logger_from_status, instead of being hardcoded to ERROR/500. Subclasses with their own specific handler (SupersetErrorException, SupersetErrorsException, CommandException) are unaffected by Flask's MRO dispatch. 500-status SupersetException subclasses keep logging at ERROR. Fixes SUPERSET-PYTHON-12EZ 🤖 Generated with Claude Code Co-Authored-By: Claude --- superset/views/error_handling.py | 15 ++++++ tests/unit_tests/views/test_error_handling.py | 54 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/superset/views/error_handling.py b/superset/views/error_handling.py index f7e2ad8bc5f3..80f9155f154e 100644 --- a/superset/views/error_handling.py +++ b/superset/views/error_handling.py @@ -171,6 +171,21 @@ def show_superset_errors(ex: SupersetErrorsException) -> FlaskResponse: logger.warning("SupersetErrorsException", exc_info=True) return json_error_response(ex.errors, status=ex.status) + @app.errorhandler(SupersetException) + def show_superset_exception(ex: SupersetException) -> FlaskResponse: + logger_func, _ = get_logger_from_status(ex.status) + logger_func(ex.message, exc_info=True) + return json_error_response( + [ + SupersetError( + message=ex.message, + error_type=SupersetErrorType.GENERIC_BACKEND_ERROR, + level=get_error_level_from_status(ex.status), + ), + ], + status=ex.status, + ) + @app.errorhandler(CSRFError) def refresh_csrf_token(ex: CSRFError) -> FlaskResponse: """Redirect to login if the CSRF token is expired""" diff --git a/tests/unit_tests/views/test_error_handling.py b/tests/unit_tests/views/test_error_handling.py index a3f46fa3a637..b01b523fec1f 100644 --- a/tests/unit_tests/views/test_error_handling.py +++ b/tests/unit_tests/views/test_error_handling.py @@ -23,6 +23,7 @@ from flask_babel import Babel from superset.errors import SupersetErrorType +from superset.exceptions import QueryObjectValidationError, SupersetException from superset.superset_typing import FlaskResponse from superset.utils import json from superset.views.error_handling import handle_api_exception, set_app_error_handlers @@ -110,3 +111,56 @@ def test_generic_exception_still_returns_original_500_shape( == SupersetErrorType.GENERIC_BACKEND_ERROR.value ) assert any(record.levelno >= logging.ERROR for record in caplog.records) + + +class TestShowSupersetException: + def _build_app_with_handlers(self) -> Flask: + # A fresh, minimal Flask app per test: `set_app_error_handlers` can + # only register handlers before the app has served its first + # request, so it can't share the module-scoped `app` fixture across + # tests in this class. + test_app = Flask(__name__) + test_app.config["DEBUG"] = False + Babel(test_app) + set_app_error_handlers(test_app) + + @test_app.route("/query-validation-error") + def query_validation_error_view() -> FlaskResponse: + raise QueryObjectValidationError("list object has no element 0") + + @test_app.route("/generic-superset-exception") + def generic_superset_exception_view() -> FlaskResponse: + raise SupersetException("boom") + + return test_app + + def test_4xx_superset_exception_returns_its_status_and_logs_at_warning( + self, caplog: pytest.LogCaptureFixture + ): + client = self._build_app_with_handlers().test_client() + + with caplog.at_level(logging.WARNING): + response = client.get("/query-validation-error") + + assert response.status_code == 400 + payload = json.loads(response.data) + assert payload["errors"][0]["message"] == "list object has no element 0" + assert not any(record.levelno >= logging.ERROR for record in caplog.records) + assert any( + record.levelno == logging.WARNING + and record.message == "list object has no element 0" + for record in caplog.records + ) + + def test_5xx_superset_exception_still_returns_500_and_logs_at_error( + self, caplog: pytest.LogCaptureFixture + ): + client = self._build_app_with_handlers().test_client() + + with caplog.at_level(logging.WARNING): + response = client.get("/generic-superset-exception") + + assert response.status_code == 500 + payload = json.loads(response.data) + assert payload["errors"][0]["message"] == "boom" + assert any(record.levelno >= logging.ERROR for record in caplog.records) From 9701464fa569c4045d30878edd38c3e0f0884a20 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Sun, 2 Aug 2026 18:11:55 +0000 Subject: [PATCH 2/2] address review feedback: preserve HTML 500 page for SupersetException The new show_superset_exception handler always returned JSON, but the uncaught-exception path it replaces (show_unexpected_exception) served the branded 500.html page for HTML-accepting browser requests. Add the same text/html branch used by show_command_errors/show_http_exception so browser clients keep getting the branded error page instead of a raw JSON payload. Co-Authored-By: Claude --- superset/views/error_handling.py | 9 +++++++++ tests/unit_tests/views/test_error_handling.py | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/superset/views/error_handling.py b/superset/views/error_handling.py index 80f9155f154e..08b55a9fb3b5 100644 --- a/superset/views/error_handling.py +++ b/superset/views/error_handling.py @@ -175,6 +175,15 @@ def show_superset_errors(ex: SupersetErrorsException) -> FlaskResponse: def show_superset_exception(ex: SupersetException) -> FlaskResponse: logger_func, _ = get_logger_from_status(ex.status) logger_func(ex.message, exc_info=True) + + if "text/html" in request.accept_mimetypes and not app.config["DEBUG"]: + path = files("superset") / "static/assets/500.html" + # Try to serve HTML file; fall back to JSON if not built + try: + return send_file(path, max_age=0), ex.status + except FileNotFoundError: + pass + return json_error_response( [ SupersetError( diff --git a/tests/unit_tests/views/test_error_handling.py b/tests/unit_tests/views/test_error_handling.py index b01b523fec1f..e1046d7e020c 100644 --- a/tests/unit_tests/views/test_error_handling.py +++ b/tests/unit_tests/views/test_error_handling.py @@ -16,6 +16,7 @@ # under the License. import logging from typing import cast +from unittest.mock import patch import pytest import sshtunnel @@ -164,3 +165,18 @@ def test_5xx_superset_exception_still_returns_500_and_logs_at_error( payload = json.loads(response.data) assert payload["errors"][0]["message"] == "boom" assert any(record.levelno >= logging.ERROR for record in caplog.records) + + def test_html_accept_serves_branded_error_page_not_raw_json(self): + client = self._build_app_with_handlers().test_client() + + with patch( + "superset.views.error_handling.send_file", + return_value=Response("500", mimetype="text/html"), + ) as mock_send_file: + response = client.get( + "/generic-superset-exception", headers={"Accept": "text/html"} + ) + + assert response.status_code == 500 + assert response.content_type.startswith("text/html") + mock_send_file.assert_called_once()