mirror of
https://github.com/apache/superset.git
synced 2026-07-27 17:12:36 +00:00
5cb0122352
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 <noreply@anthropic.com>
102 lines
3.8 KiB
Python
102 lines
3.8 KiB
Python
# Licensed to the Apache Software Foundation (ASF) under one
|
|
# or more contributor license agreements. See the NOTICE file
|
|
# distributed with this work for additional information
|
|
# regarding copyright ownership. The ASF licenses this file
|
|
# to you under the Apache License, Version 2.0 (the
|
|
# "License"); you may not use this file except in compliance
|
|
# with the License. You may obtain a copy of the License at
|
|
#
|
|
# http://www.apache.org/licenses/LICENSE-2.0
|
|
#
|
|
# Unless required by applicable law or agreed to in writing,
|
|
# software distributed under the License is distributed on an
|
|
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
|
# KIND, either express or implied. See the License for the
|
|
# specific language governing permissions and limitations
|
|
# under the License.
|
|
from typing import Any
|
|
from unittest.mock import patch
|
|
|
|
import rison
|
|
|
|
from superset.exceptions import SupersetException
|
|
from tests.unit_tests.conftest import with_feature_flags
|
|
|
|
|
|
@with_feature_flags(ALERT_REPORTS=True)
|
|
@patch("superset.reports.api.get_channels_with_search")
|
|
def test_slack_channels_success(
|
|
mock_search: Any,
|
|
client: Any,
|
|
full_api_access: None,
|
|
) -> None:
|
|
mock_search.return_value = [{"id": "C123", "name": "general"}]
|
|
params = rison.dumps({})
|
|
rv = client.get(f"/api/v1/report/slack_channels/?q={params}")
|
|
assert rv.status_code == 200
|
|
data = rv.json
|
|
assert data["result"] == [{"id": "C123", "name": "general"}]
|
|
assert data["count"] == 1
|
|
|
|
|
|
@with_feature_flags(ALERT_REPORTS=True)
|
|
@patch("superset.reports.api.get_channels_with_search")
|
|
def test_slack_channels_paginates(
|
|
mock_search: Any,
|
|
client: Any,
|
|
full_api_access: None,
|
|
) -> None:
|
|
# A large workspace: the endpoint must return only the requested page while
|
|
# reporting the full count, so the browser never receives every channel.
|
|
mock_search.return_value = [
|
|
{"id": f"C{i}", "name": f"channel-{i}"} for i in range(250)
|
|
]
|
|
params = rison.dumps({"page": 1, "page_size": 100})
|
|
rv = client.get(f"/api/v1/report/slack_channels/?q={params}")
|
|
assert rv.status_code == 200
|
|
data = rv.json
|
|
assert data["count"] == 250
|
|
assert len(data["result"]) == 100
|
|
assert data["result"][0] == {"id": "C100", "name": "channel-100"}
|
|
|
|
|
|
@with_feature_flags(ALERT_REPORTS=True)
|
|
@patch("superset.reports.api.get_channels_with_search")
|
|
def test_slack_channels_page_without_page_size_returns_all(
|
|
mock_search: Any,
|
|
client: Any,
|
|
full_api_access: None,
|
|
) -> None:
|
|
# Pagination only kicks in when both page and page_size are supplied; a page
|
|
# without page_size falls through to the full (unsliced) list.
|
|
mock_search.return_value = [
|
|
{"id": f"C{i}", "name": f"channel-{i}"} for i in range(30)
|
|
]
|
|
params = rison.dumps({"page": 1})
|
|
rv = client.get(f"/api/v1/report/slack_channels/?q={params}")
|
|
assert rv.status_code == 200
|
|
assert rv.json["count"] == 30
|
|
assert len(rv.json["result"]) == 30
|
|
|
|
|
|
@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]
|