fix(reports): keep ERROR for transient Slack listing failures, WARNING for client errors

Split the slack_channels handler so SlackChannelListingClientError (permanent
token/client-setup failures) logs at WARNING while the base SupersetException
path, including the transient SlackChannelListingError case, stays at ERROR so
Slack outages still emit an actionable signal.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

superset/reports/api.py

@@ -63,7 +63,10 @@ from superset.reports.schemas import (
     ReportScheduleSubscribeSchema,
 )
 from superset.subjects.filters import FilterRelatedSubjects, subject_type_filter
-from superset.utils.slack import get_channels_with_search
+from superset.utils.slack import (
+    get_channels_with_search,
+    SlackChannelListingClientError,
+)
 from superset.views.base_api import (
     BaseSupersetModelRestApi,
     RelatedFieldFilter,
@@ -716,7 +719,15 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
                 start = page * page_size
                 channels = channels[start : start + page_size]
             return self.response(200, count=count, result=channels)
+        except SlackChannelListingClientError as ex:
+            # Permanent token/client-setup failures are expected, already-handled
+            # noise (e.g. a revoked bot token), so log at WARNING to keep Sentry
+            # clear of an actionable-looking signal.
+            logger.warning("Error fetching slack channels %s", str(ex))
+            return self.response_422(message=str(ex))
         except SupersetException as ex:
+            # Transient listing failures (rate limits, transport errors) mean
+            # Slack is unavailable, so keep ERROR to preserve an actionable signal.
             logger.error("Error fetching slack channels %s", str(ex))
             return self.response_422(message=str(ex))
 

superset/utils/slack.py

@@ -141,6 +141,10 @@ _TRANSIENT_SLACK_API_ERROR_CODES = frozenset(
     }
 )
 
+_AUTH_ERROR_CODES = frozenset(
+    {"not_authed", "invalid_auth", "account_inactive", "token_revoked", "token_expired"}
+)
+
 SLACK_TRANSIENT_TRANSPORT_ERRORS: tuple[type[Exception], ...] = (
     SlackClientNotConnectedError,
     URLError,
@@ -392,12 +396,24 @@ def _get_channels(team_id: Optional[str] = None) -> list[SlackChannel]:
         )
         return channels
     except SlackApiError as ex:
-        logger.error(
-            "Failed to fetch Slack channels after %d pages: %s",
-            page_count,
-            str(ex),
-            exc_info=True,
-        )
+        # Only bot-token auth failures (invalid/revoked/deactivated) are the
+        # expected, already-handled multi-tenant condition this is meant to
+        # quiet down. Rate limits and Slack server/API errors are actionable
+        # outages, so they keep ERROR-level logging with a traceback.
+        error_code = get_slack_api_error_code(ex)
+        if error_code in _AUTH_ERROR_CODES:
+            logger.warning(
+                "Failed to fetch Slack channels after %d pages: %s",
+                page_count,
+                str(ex),
+            )
+        else:
+            logger.error(
+                "Failed to fetch Slack channels after %d pages: %s",
+                page_count,
+                str(ex),
+                exc_info=True,
+            )
         raise
 
 

tests/unit_tests/reports/api_test.py

@@ -19,7 +19,10 @@ from unittest.mock import patch
 
 import rison
 
-from superset.exceptions import SupersetException
+from superset.utils.slack import (
+    SlackChannelListingClientError,
+    SlackChannelListingError,
+)
 from tests.unit_tests.conftest import with_feature_flags
 
 
@@ -80,14 +83,43 @@ def test_slack_channels_page_without_page_size_returns_all(
 
 
 @with_feature_flags(ALERT_REPORTS=True)
+@patch("superset.reports.api.logger")
 @patch("superset.reports.api.get_channels_with_search")
-def test_slack_channels_handles_superset_exception(
+def test_slack_channels_client_error_logs_warning(
     mock_search: Any,
+    logger_mock: Any,
     client: Any,
     full_api_access: None,
 ) -> None:
-    mock_search.side_effect = SupersetException("Slack API error")
+    # A permanent token/client-setup failure (e.g. a revoked bot token) is
+    # expected, already-handled noise, so it must be logged at WARNING, not
+    # ERROR, to avoid polluting Sentry with an actionable-looking signal.
+    mock_search.side_effect = SlackChannelListingClientError("Slack API error")
     params = rison.dumps({})
     rv = client.get(f"/api/v1/report/slack_channels/?q={params}")
     assert rv.status_code == 422
     assert "Slack API error" in rv.json["message"]
+    logger_mock.error.assert_not_called()
+    logger_mock.warning.assert_called_once()
+    assert "Slack API error" in logger_mock.warning.call_args.args[1]
+
+
+@with_feature_flags(ALERT_REPORTS=True)
+@patch("superset.reports.api.logger")
+@patch("superset.reports.api.get_channels_with_search")
+def test_slack_channels_transient_error_logs_error(
+    mock_search: Any,
+    logger_mock: Any,
+    client: Any,
+    full_api_access: None,
+) -> None:
+    # A transient listing failure (rate limits, transport errors) means Slack is
+    # unavailable, so it must stay ERROR to preserve an actionable signal.
+    mock_search.side_effect = SlackChannelListingError("Slack API error")
+    params = rison.dumps({})
+    rv = client.get(f"/api/v1/report/slack_channels/?q={params}")
+    assert rv.status_code == 422
+    assert "Slack API error" in rv.json["message"]
+    logger_mock.warning.assert_not_called()
+    logger_mock.error.assert_called_once()
+    assert "Slack API error" in logger_mock.error.call_args.args[1]

tests/unit_tests/utils/slack_test.py

@@ -237,6 +237,62 @@ class TestGetChannelsWithSearch:
 The server responded with: missing scope: channels:read"""
         )
 
+    @pytest.mark.parametrize(
+        "error_code",
+        [
+            "not_authed",
+            "invalid_auth",
+            "account_inactive",
+            "token_revoked",
+            "token_expired",
+        ],
+    )
+    def test_logs_slack_api_error_at_warning_not_error(self, error_code: str, mocker):
+        """An expired/revoked/inactive bot token is an expected multi-tenant
+        config state that is already handled end-to-end (re-raised as a
+        ``SupersetException`` and turned into a 422), so it should be logged
+        at WARNING, not ERROR, to avoid polluting Sentry."""
+        from superset.exceptions import SupersetException
+
+        mock_client = mocker.Mock()
+        mock_client.conversations_list.side_effect = SlackApiError(
+            message="foo", response={"ok": False, "error": error_code}
+        )
+        mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client)
+        logger_mock = mocker.patch("superset.utils.slack.logger")
+
+        with pytest.raises(SupersetException):
+            get_channels_with_search()
+
+        logger_mock.error.assert_not_called()
+        logger_mock.warning.assert_called_once()
+        assert "Failed to fetch Slack channels" in logger_mock.warning.call_args.args[0]
+        assert not logger_mock.warning.call_args.kwargs.get("exc_info")
+
+    @pytest.mark.parametrize("error_code", ["ratelimited", "internal_error", ""])
+    def test_logs_non_auth_slack_api_error_at_error_with_traceback(
+        self, error_code: str, mocker
+    ):
+        """Rate limits and Slack server/API errors are actionable outages, not
+        the expected auth-noise condition — they must keep ERROR-level logging
+        with a traceback so they still generate a Sentry event."""
+        from superset.exceptions import SupersetException
+
+        mock_client = mocker.Mock()
+        mock_client.conversations_list.side_effect = SlackApiError(
+            message="foo", response={"ok": False, "error": error_code}
+        )
+        mocker.patch("superset.utils.slack.get_slack_client", return_value=mock_client)
+        logger_mock = mocker.patch("superset.utils.slack.logger")
+
+        with pytest.raises(SupersetException):
+            get_channels_with_search()
+
+        logger_mock.warning.assert_not_called()
+        logger_mock.error.assert_called_once()
+        assert "Failed to fetch Slack channels" in logger_mock.error.call_args.args[0]
+        assert logger_mock.error.call_args.kwargs.get("exc_info") is True
+
     @pytest.mark.parametrize(
         ("error_code", "expected_exception"),
         [