chore: add logging for dashboards/get warnings (apache#30365)

superset/commands/database/exceptions.py

@@ -202,3 +202,15 @@ class DatabaseOfflineError(SupersetErrorException):
 
 class InvalidParametersError(SupersetErrorsException):
     status = 422
+
+
+class DatasetValidationError(CommandException):
+    status = 422
+
+    def __init__(self, err: Exception) -> None:
+        super().__init__(
+            _(
+                "Dataset schema is invalid, caused by: %(error)s",
+                error=str(err),
+            )
+        )

superset/dashboards/api.py

@@ -56,6 +56,7 @@
 from superset.commands.dashboard.permalink.create import CreateDashboardPermalinkCommand
 from superset.commands.dashboard.unfave import DelFavoriteDashboardCommand
 from superset.commands.dashboard.update import UpdateDashboardCommand
+from superset.commands.database.exceptions import DatasetValidationError
 from superset.commands.exceptions import TagForbiddenError
 from superset.commands.importers.exceptions import NoValidFilesFoundError
 from superset.commands.importers.v1.utils import get_contents_from_bundle
@@ -115,6 +116,7 @@
     requires_json,
     statsd_metrics,
 )
+from superset.views.error_handling import handle_api_exception
 from superset.views.filters import (
     BaseFilterRelatedRoles,
     BaseFilterRelatedUsers,
@@ -369,7 +371,7 @@ def get(
 
     @expose("/<id_or_slug>/datasets", methods=("GET",))
     @protect()
-    @safe
+    @handle_api_exception
     @statsd_metrics
     @event_logger.log_this_with_context(
         action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_datasets",
@@ -417,15 +419,7 @@ def get_datasets(self, id_or_slug: str) -> Response:
             ]
             return self.response(200, result=result)
         except (TypeError, ValueError) as err:
-            return self.response_400(
-                message=gettext(
-                    "Dataset schema is invalid, caused by: %(error)s", error=str(err)
-                )
-            )
-        except DashboardAccessDeniedError:
-            return self.response_403()
-        except DashboardNotFoundError:
-            return self.response_404()
+            raise DatasetValidationError(err) from err
 
     @expose("/<id_or_slug>/tabs", methods=("GET",))
     @protect()

superset/views/error_handling.py

@@ -45,6 +45,7 @@
 )
 from superset.superset_typing import FlaskResponse
 from superset.utils import core as utils, json
+from superset.utils.log import get_logger_from_status
 
 if typing.TYPE_CHECKING:
     from superset.views.base import BaseSupersetView
@@ -108,8 +109,8 @@ def wraps(self: BaseSupersetView, *args: Any, **kwargs: Any) -> FlaskResponse:
             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)
+            logger_func, _ = get_logger_from_status(ex.status)
+            logger_func(ex.message, exc_info=True)
             return json_error_response(
                 utils.error_msg_from_exception(ex), status=ex.status
             )

tests/integration_tests/dashboards/api_tests.py

@@ -270,7 +270,8 @@ def create_dashboards_some_with_tags(self, create_custom_tags):  # noqa: F811
             db.session.commit()
 
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
-    def test_get_dashboard_datasets(self):
+    @patch("superset.utils.log.logger")
+    def test_get_dashboard_datasets(self, logger_mock):
         self.login(ADMIN_USERNAME)
         uri = "api/v1/dashboard/world_health/datasets"
         response = self.get_assert_metric(uri, "get_datasets")
@@ -283,6 +284,7 @@ def test_get_dashboard_datasets(self):
         self.assertEqual(actual_dataset_ids, expected_dataset_ids)
         expected_values = [0, 1] if backend() == "presto" else [0, 1, 2]
         self.assertEqual(result[0]["column_types"], expected_values)
+        logger_mock.warning.assert_not_called()
 
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @patch("superset.dashboards.schemas.security_manager.has_guest_access")
@@ -303,11 +305,30 @@ def test_get_dashboard_datasets_as_guest(self, is_guest_user, has_guest_access):
                 assert excluded_key not in dataset
 
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
-    def test_get_dashboard_datasets_not_found(self):
+    @patch("superset.utils.log.logger")
+    def test_get_dashboard_datasets_not_found(self, logger_mock):
         self.login(ALPHA_USERNAME)
         uri = "api/v1/dashboard/not_found/datasets"
         response = self.get_assert_metric(uri, "get_datasets")
         self.assertEqual(response.status_code, 404)
+        logger_mock.warning.assert_called_once_with(
+            "Dashboard not found.", exc_info=True
+        )
+
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    @patch("superset.utils.log.logger")
+    @patch("superset.daos.dashboard.DashboardDAO.get_datasets_for_dashboard")
+    def test_get_dashboard_datasets_invalid_schema(
+        self, dashboard_datasets_mock, logger_mock
+    ):
+        dashboard_datasets_mock.side_effect = TypeError("Invalid schema")
+        self.login(ADMIN_USERNAME)
+        uri = "api/v1/dashboard/world_health/datasets"
+        response = self.get_assert_metric(uri, "get_datasets")
+        self.assertEqual(response.status_code, 422)
+        logger_mock.warning.assert_called_once_with(
+            "Dataset schema is invalid, caused by: Invalid schema", exc_info=True
+        )
 
     @pytest.mark.usefixtures("create_dashboards")
     def test_get_gamma_dashboard_datasets(self):