eschutho opened a new pull request, #42484: URL: https://github.com/apache/superset/pull/42484
### SUMMARY Two Sentry issues in org `preset-inc`, same root cause: - [SUPERSET-PYTHON-P8F](https://preset-inc.sentry.io/issues/) — "Error fetching slack channels ... not_authed" — **7146 events, 90 users** - [SUPERSET-PYTHON-Y7R](https://preset-inc.sentry.io/issues/) — "SlackApiError ... invalid_auth" — **2275 events, 1 user** **Root cause:** `ReportScheduleRestApi.slack_channels()` lets a user list Slack channels when configuring a report/alert Slack recipient. When a workspace's Slack bot token is invalid or revoked (Slack API returns `not_authed` or `invalid_auth` — an expected multi-tenant config state, not a Superset code bug), the request path already handles it correctly end-to-end: 1. `superset/utils/slack.py::_get_channels()` catches `SlackApiError` and logs it (previously at `ERROR` with `exc_info=True`) before re-raising. 2. That propagates up through `get_channels_with_search()`, which wraps it into a `SupersetException`. 3. `superset/reports/api.py`'s `slack_channels()` handler catches `SupersetException`, logs it (previously also at `ERROR`), and correctly returns `self.response_422(message=str(ex))`. So the exact same underlying, fully-handled condition was logged at `ERROR` twice per request — once with a full traceback — even though the endpoint already returns a correct 4xx. Sentry's default `LoggingIntegration` (event_level=ERROR) captures both as separate issues, generating ~9400 events of noise for a condition that is expected in a multi-tenant deployment (revoked/invalid per-workspace bot tokens) and already surfaced to the end user via the 422 response. This is the same bug class as previously-fixed MANAGER-PYTHON-2ZF, MANAGER-PYTHON-374, and SUPERSET-PYTHON-XVB/YYS in this codebase: an already-handled, expected condition logged loudly enough to pollute Sentry. **Fix:** downgrade both log calls to `logger.warning`: - `superset/utils/slack.py` (`_get_channels`, `except SlackApiError`): `logger.error(...)` → `logger.warning(...)`, and drop `exc_info=True` — this matches the WARNING-level precedent already set by `should_use_v2_api()` in the same file for Slack auth/scope errors. - `superset/reports/api.py` (`slack_channels`, `except SupersetException`): `logger.error(...)` → `logger.warning(...)`. ### TRADEOFFS This is a **log-severity-only** change. There is no change to: - exception types raised or caught - the 422 response contract (`response_422(message=str(ex))` is untouched) - control flow / re-raise behavior - error handling scope (the broader `except SlackApiError` / `except SlackClientError` blocks in `get_channels_with_search()` that wrap into `SupersetException` are untouched) The only behavioral effect is that these two already-handled conditions stop being captured as ERROR-level Sentry events (and stop carrying a full traceback), while remaining visible in application logs at WARNING level and in the API response to the caller. ### TESTING INSTRUCTIONS Added/updated unit tests asserting the log call is `logger.warning` (not `.error`) and that the response/behavior is unchanged: - `tests/unit_tests/utils/slack_test.py::TestGetChannelsWithSearch::test_logs_slack_api_error_at_warning_not_error` (new) - `tests/unit_tests/reports/api_test.py::test_slack_channels_handles_superset_exception` (extended with logger assertions) Test run: ``` tests/unit_tests/utils/slack_test.py .......................... [100%] tests/unit_tests/reports/api_test.py .......................... [100%] tests/unit_tests/reports/notifications/slack_tests.py ........ [100%] 72 passed, 1 warning in 2.89s ``` Lint: ``` ruff check superset/utils/slack.py superset/reports/api.py tests/unit_tests/utils/slack_test.py tests/unit_tests/reports/api_test.py All checks passed! ruff format --check <same files> 4 files already formatted ``` `mypy` on the two changed source files reports no new errors attributable to this change (pre-existing repo-wide errors in unrelated transitively-imported modules are unaffected). ### ADDITIONAL INFORMATION - [ ] Has associated issue: https://app.shortcut.com/preset/story/115301 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API Fixes SUPERSET-PYTHON-P8F Fixes SUPERSET-PYTHON-Y7R 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
