This is an automated email from the ASF dual-hosted git repository. eschutho pushed a commit to branch fix-slack-channels-auth-noise in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5cb012235224643e80cbb62063a0ad0a027a3085 Author: Elizabeth Thompson <[email protected]> AuthorDate: Mon Jul 27 15:16:50 2026 +0000 fix(reports): downgrade Slack channel-fetch auth-error logging to WARNING (SC-115301) Slack bot tokens can be invalid or revoked per-workspace, which is an expected multi-tenant configuration state, not a Superset bug. When this happens, `_get_channels()` in `superset/utils/slack.py` already catches `SlackApiError` and re-raises after logging (previously at ERROR with a full traceback), which propagates through `get_channels_with_search()` as a `SupersetException`, which `ReportScheduleRestApi.slack_channels()` in `superset/reports/api.py` already catches and correctly turns into a 422 (previously also logging at ERROR). The same already-handled condition was therefore logged at ERROR twice per request, generating two separate Sentry issues for what is fully handled, expected behavior with no behavior change needed. Downgrade both log calls to `logger.warning` (dropping `exc_info=True` on the lower one, matching the WARNING-level precedent already set by `should_use_v2_api()` in the same file) so Sentry's default ERROR-level capture stops firing on this expected condition. The 422 response contract, exception handling, and control flow are unchanged. Fixes SUPERSET-PYTHON-P8F Fixes SUPERSET-PYTHON-Y7R Co-Authored-By: Claude <[email protected]> --- superset/reports/api.py | 2 +- superset/utils/slack.py | 3 +-- tests/unit_tests/reports/api_test.py | 8 ++++++++ tests/unit_tests/utils/slack_test.py | 19 +++++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/superset/reports/api.py b/superset/reports/api.py index 01e7b1226b2..da734a3b315 100644 --- a/superset/reports/api.py +++ b/superset/reports/api.py @@ -700,7 +700,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi): channels = channels[start : start + page_size] return self.response(200, count=count, result=channels) except SupersetException as ex: - logger.error("Error fetching slack channels %s", str(ex)) + logger.warning("Error fetching slack channels %s", str(ex)) return self.response_422(message=str(ex)) @expose("/<int:pk>/execute", methods=("POST",)) diff --git a/superset/utils/slack.py b/superset/utils/slack.py index 0e0e422d525..8f7c9015f43 100644 --- a/superset/utils/slack.py +++ b/superset/utils/slack.py @@ -185,11 +185,10 @@ def _get_channels( ) return channels except SlackApiError as ex: - logger.error( + logger.warning( "Failed to fetch Slack channels after %d pages: %s", page_count, str(ex), - exc_info=True, ) raise diff --git a/tests/unit_tests/reports/api_test.py b/tests/unit_tests/reports/api_test.py index 280cb108ba8..8688497e2a7 100644 --- a/tests/unit_tests/reports/api_test.py +++ b/tests/unit_tests/reports/api_test.py @@ -80,14 +80,22 @@ 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( mock_search: Any, + logger_mock: Any, client: Any, full_api_access: None, ) -> None: + # A SupersetException here typically wraps an already-handled Slack auth + # error (e.g. a revoked bot token), so it must be logged at WARNING, not + # ERROR, to avoid polluting Sentry with an expected, already-handled state. mock_search.side_effect = SupersetException("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] diff --git a/tests/unit_tests/utils/slack_test.py b/tests/unit_tests/utils/slack_test.py index e2da1cd16cc..a1b3baf34a4 100644 --- a/tests/unit_tests/utils/slack_test.py +++ b/tests/unit_tests/utils/slack_test.py @@ -164,6 +164,25 @@ class TestGetChannelsWithSearch: The server responded with: missing scope: channels:read""" ) + def test_logs_slack_api_error_at_warning_not_error(self, mocker): + """An expired/revoked bot token (``not_authed``/``invalid_auth``) 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("foo", "not_authed") + 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] + @pytest.mark.parametrize( "types, expected_channel_ids", [
