From 5243fcafa1fcf275e05ee2bac4753eab3f1906f3 Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Thu, 25 Jun 2026 12:24:28 +0300 Subject: [PATCH] fix(slack): surface retryable + Retry-After on send() rate-limit errors (#46762) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack's send() caught all exceptions and returned a bare SendResult(success=False) — never setting retryable=True or extracting the server's Retry-After header. When Slack returned a 429 rate-limit error, the base _send_with_retry() layer saw retryable=False and did not retry, silently dropping remaining message chunks. Reuse the existing _is_retryable_upload_error() helper (which already detects 429, 500+, and connection-type errors) to set retryable=True, and extract the Retry-After header from the SlackApiError response when present so the base retry layer honors Slack's backoff schedule instead of its own default. Sibling of the Telegram FloodWait fix (PR #46762 / commit 404b06ac4) which added the SendResult.retry_after plumbing to the base layer. Adds five regression tests covering 429 with/without Retry-After, 500 server errors, 403 non-retryable errors, and connection errors. --- plugins/platforms/slack/adapter.py | 18 ++- tests/gateway/test_slack_send_retry.py | 152 +++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/gateway/test_slack_send_retry.py diff --git a/plugins/platforms/slack/adapter.py b/plugins/platforms/slack/adapter.py index 9329942f6f20..a842b3672ae1 100644 --- a/plugins/platforms/slack/adapter.py +++ b/plugins/platforms/slack/adapter.py @@ -1914,7 +1914,23 @@ class SlackAdapter(BasePlatformAdapter): if thread_ts: await self.stop_typing(chat_id, metadata=metadata) logger.error("[Slack] Send error: %s", e, exc_info=True) - return SendResult(success=False, error=str(e)) + _retryable = self._is_retryable_upload_error(e) + _retry_after = None + if _retryable: + _resp = getattr(e, "response", None) + if _resp is not None: + try: + _ra = getattr(_resp, "headers", {}).get("Retry-After") + if _ra is not None: + _retry_after = float(_ra) + except (TypeError, ValueError, AttributeError): + pass + return SendResult( + success=False, + error=str(e), + retryable=_retryable, + retry_after=_retry_after, + ) async def send_private_notice( self, diff --git a/tests/gateway/test_slack_send_retry.py b/tests/gateway/test_slack_send_retry.py new file mode 100644 index 000000000000..c6cb13582d25 --- /dev/null +++ b/tests/gateway/test_slack_send_retry.py @@ -0,0 +1,152 @@ +"""Regression: Slack send() must surface retryable + retry_after on 429. + +The Telegram adapter (PR #46762) extracts retry_after from FloodWait errors +and sets retryable=True so the base _send_with_retry() layer can honor the +server-requested backoff. Slack's send() caught all exceptions and returned +a bare SendResult(success=False) with retryable=False — so 429 rate-limit +errors were never retried and remaining message chunks were silently dropped. + +These tests verify the Slack adapter now: + 1. Sets retryable=True for 429 / 500+ / connection errors + 2. Extracts the Retry-After header when present + 3. Leaves non-retryable errors (403, 404, etc.) unchanged +""" + +import sys +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from gateway.config import PlatformConfig + + +# --------------------------------------------------------------------------- +# Ensure slack mocks are in place before importing the adapter +# --------------------------------------------------------------------------- + +def _ensure_slack_mock(): + if "slack_bolt" in sys.modules and hasattr(sys.modules["slack_bolt"], "__file__"): + return + slack_bolt = MagicMock() + slack_bolt.async_app.AsyncApp = MagicMock + slack_bolt.adapter.socket_mode.async_handler.AsyncSocketModeHandler = MagicMock + slack_sdk = MagicMock() + slack_sdk.web.async_client.AsyncWebClient = MagicMock + for name, mod in [ + ("slack_bolt", slack_bolt), + ("slack_bolt.async_app", slack_bolt.async_app), + ("slack_bolt.adapter", slack_bolt.adapter), + ("slack_bolt.adapter.socket_mode", slack_bolt.adapter.socket_mode), + ("slack_bolt.adapter.socket_mode.async_handler", slack_bolt.adapter.socket_mode.async_handler), + ("slack_sdk", slack_sdk), + ("slack_sdk.web", slack_sdk.web), + ("slack_sdk.web.async_client", slack_sdk.web.async_client), + ]: + sys.modules.setdefault(name, mod) + + +_ensure_slack_mock() + +from plugins.platforms.slack.adapter import SlackAdapter + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_adapter(): + config = PlatformConfig(enabled=True, token="xoxb-fake") + a = SlackAdapter(config) + a._app = MagicMock() + a._bot_user_id = "U_BOT" + a._running = True + return a + + +def _slack_api_error(status_code: int, retry_after: str = None): + """Simulate a SlackApiError with response status and optional Retry-After.""" + headers = {} + if retry_after is not None: + headers["Retry-After"] = retry_after + resp = SimpleNamespace( + status_code=status_code, + headers=headers, + ) + exc = Exception(f"The request to the Slack API failed. (status: {status_code})") + exc.response = resp + return exc + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + +class TestSlackSendRetryable: + @pytest.mark.asyncio + async def test_429_returns_retryable_with_retry_after(self): + adapter = _make_adapter() + client = AsyncMock() + client.chat_postMessage = AsyncMock( + side_effect=_slack_api_error(429, retry_after="30") + ) + adapter._get_client = lambda cid: client + + result = await adapter.send("C123", "hello") + assert not result.success + assert result.retryable is True + assert result.retry_after == 30.0 + + @pytest.mark.asyncio + async def test_429_without_retry_after_header(self): + adapter = _make_adapter() + client = AsyncMock() + client.chat_postMessage = AsyncMock( + side_effect=_slack_api_error(429) + ) + adapter._get_client = lambda cid: client + + result = await adapter.send("C123", "hello") + assert not result.success + assert result.retryable is True + assert result.retry_after is None + + @pytest.mark.asyncio + async def test_500_is_retryable_no_retry_after(self): + adapter = _make_adapter() + client = AsyncMock() + client.chat_postMessage = AsyncMock( + side_effect=_slack_api_error(500) + ) + adapter._get_client = lambda cid: client + + result = await adapter.send("C123", "hello") + assert not result.success + assert result.retryable is True + assert result.retry_after is None + + @pytest.mark.asyncio + async def test_403_is_not_retryable(self): + adapter = _make_adapter() + client = AsyncMock() + client.chat_postMessage = AsyncMock( + side_effect=_slack_api_error(403) + ) + adapter._get_client = lambda cid: client + + result = await adapter.send("C123", "hello") + assert not result.success + assert result.retryable is False + + @pytest.mark.asyncio + async def test_connection_error_is_retryable(self): + adapter = _make_adapter() + client = AsyncMock() + client.chat_postMessage = AsyncMock( + side_effect=ConnectionError("Connection reset by peer") + ) + adapter._get_client = lambda cid: client + + result = await adapter.send("C123", "hello") + assert not result.success + assert result.retryable is True