fix(errors): map uncaught SupersetException status/log-level correctly

superset/views/error_handling.py

@@ -171,6 +171,30 @@ 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)
+
+        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(
+                    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"""

tests/unit_tests/views/test_error_handling.py

@@ -16,13 +16,15 @@
 # under the License.
 import logging
 from typing import cast
+from unittest.mock import patch
 
 import pytest
 import sshtunnel
 from flask import Flask, Response
 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 +112,71 @@ 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)
+
+    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("<html>500</html>", 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()