aminghadersohi opened a new pull request, #35588:
URL: https://github.com/apache/superset/pull/35588
### SUMMARY
Fixes the issue where Slack API rate limit errors (HTTP 429) were being
swallowed or masked by generic error messages in the `slack-sdk` library's
retry handler. This change improves observability and reliability of Slack
integrations by adding explicit error detection and detailed logging.
**Root Cause**: The `RateLimitErrorRetryHandler` in the slack-sdk library
can re-raise errors without proper context when certain conditions are met.
When these errors bubble up to generic exception handlers, they get logged as
"Failed to send a request to Slack API server" instead of clearly indicating a
rate limit issue.
**Changes**:
- Increased retry count from 2 to 4 to provide more buffer before failure
- Added explicit error handling for HTTP 429 rate limit responses in
`get_channels()`
- Enhanced logging to include:
- Retry-After header value from Slack API
- Indication that retry handler may have failed or exhausted retries
- Full error context for debugging
- Added client initialization logging for debugging
- Fixed edge case where `ex.response` could be a string instead of response
object using `hasattr()` check
**Impact**: Rate limit errors will now be clearly visible in monitoring
systems like Datadog with actionable information (Retry-After values, retry
counts), making it much easier to diagnose and respond to Slack API rate
limiting issues.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A - Backend logging improvement
### TESTING INSTRUCTIONS
1. All existing unit tests pass: `pytest
tests/unit_tests/utils/slack_test.py`
2. Pre-commit hooks pass: `pre-commit run --files superset/utils/slack.py`
3. To manually test rate limiting:
- Configure Slack integration in Superset
- Trigger multiple rapid Slack API calls (e.g., by editing multiple alert
configurations)
- Observe error logs - they should now clearly indicate "Slack API rate
limit exceeded (HTTP 429)" with Retry-After information instead of generic
errors
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
