fix: propagate Slack reply errors (#1358)

* fix: propagate Slack reply errors

Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com>

* fix: classify Slack 429 as rate_limited with retry-after

Slack chat.postMessage rate limiting returns HTTP 429, which raise_for_status
turned into a generic http_error and told the agent to retry immediately,
ignoring Slack's retry window. Special-case 429 (threading Retry-After) and
normalize the ratelimited body code before the generic HTTP path so the
existing rate_limited hint actually fires.

---------

Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
Co-authored-by: Johannes du Plessis <51395795+johannes117@users.noreply.github.com>
Co-authored-by: Johannes du Plessis <johannes@langchain.dev>
This commit is contained in:
open-swe[bot] 2026-05-28 23:11:42 +00:00 • committed by GitHub
parent a361ee8f2e
commit eb92947806
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 281 additions and 19 deletions

View file

@ -166,6 +166,7 @@ Posts a comment to a Linear ticket given a `ticket_id`. Call this after opening/
#### `slack_thread_reply` #### `slack_thread_reply`
Posts a message to the active Slack thread. Use this for clarifying questions, mid-run progress updates, and final summaries when the task was triggered from Slack. You can call it multiple times during a run — if you're about to do something long-running (cloning a large repo, big refactors, running heavy test suites), post a short status update first so the user knows what's happening. Always end the run with a final reply that summarizes what you did or answers the question. Do not post a status reply before quick, single-tool answers — only when the user would otherwise be left waiting. Posts a message to the active Slack thread. Use this for clarifying questions, mid-run progress updates, and final summaries when the task was triggered from Slack. You can call it multiple times during a run — if you're about to do something long-running (cloning a large repo, big refactors, running heavy test suites), post a short status update first so the user knows what's happening. Always end the run with a final reply that summarizes what you did or answers the question. Do not post a status reply before quick, single-tool answers — only when the user would otherwise be left waiting.
If `slack_thread_reply` returns `success: False`, treat it like any other tool failure. Read the `slack_error` and `hint` fields. Never emit a final response message as if the user received it when the Slack post failed.
Format messages using Slack's mrkdwn format, NOT standard Markdown. Format messages using Slack's mrkdwn format, NOT standard Markdown.
Key differences: *bold*, _italic_, ~strikethrough~, <url|link text>, Key differences: *bold*, _italic_, ~strikethrough~, <url|link text>,
bullet lists with "• ", ```code blocks```, > blockquotes. bullet lists with "• ", ```code blocks```, > blockquotes.

View file

@ -49,13 +49,40 @@ def slack_thread_reply(message: str) -> dict[str, Any]:
return {"success": False, "error": "Message cannot be empty"} return {"success": False, "error": "Message cannot be empty"}
message = convert_mentions_to_slack_format(message) message = convert_mentions_to_slack_format(message)
message_ts = asyncio.run(_post_and_store_mapping(channel_id, thread_ts, message)) message_ts, slack_error = asyncio.run(_post_and_store_mapping(channel_id, thread_ts, message))
return {"success": message_ts is not None} if message_ts is None:
return {
"success": False,
"error": slack_error or "post failed",
"slack_error": slack_error,
"message_chars": len(message),
"hint": _slack_reply_failure_hint(slack_error),
}
return {"success": True}
async def _post_and_store_mapping(channel_id: str, thread_ts: str, message: str) -> str | None: def _slack_reply_failure_hint(slack_error: str | None) -> str:
message_ts = await post_slack_thread_reply_with_ts(channel_id, thread_ts, message) if slack_error == "msg_too_long":
return "Slack rejected the message as too long; retry with a shorter message."
if slack_error in {"channel_not_found", "not_in_channel"}:
return "Slack rejected the channel; do not retry. Surface the failure to the user via the trace output instead."
if slack_error and slack_error.startswith("rate_limited"):
retry_after = slack_error.partition(":")[2].strip()
if retry_after:
return f"Slack rate limited the request; wait at least {retry_after}s before retrying, or surface the failure to the user via the trace output."
return "Slack rate limited the request; wait before retrying, or surface the failure to the user via the trace output."
if slack_error == "missing_slack_bot_token":
return "Slack bot token is missing; do not retry. Surface the failure to the user via the trace output instead."
if slack_error and slack_error.startswith("http_error:"):
return "Slack posting hit an HTTP error; retry once, then surface the failure to the user via the trace output."
return "Slack post failed; retry once with a concise message or surface the failure to the user via the trace output."
async def _post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
message_ts, slack_error = await post_slack_thread_reply_with_ts(channel_id, thread_ts, message)
if message_ts: if message_ts:
langgraph_client = get_client(url=LANGGRAPH_URL) langgraph_client = get_client(url=LANGGRAPH_URL)
await store_slack_message_run_mapping(langgraph_client, channel_id, thread_ts, message_ts) await store_slack_message_run_mapping(langgraph_client, channel_id, thread_ts, message_ts)
return message_ts return message_ts, slack_error

View file

@ -330,10 +330,10 @@ async def post_slack_thread_reply_with_ts(
*, *,
unfurl_links: bool = True, unfurl_links: bool = True,
unfurl_media: bool = True, unfurl_media: bool = True,
) -> str | None: ) -> tuple[str | None, str | None]:
"""Post a reply in a Slack thread and return its Slack timestamp.""" """Post a reply in a Slack thread and return its Slack timestamp and error."""
if not SLACK_BOT_TOKEN: if not SLACK_BOT_TOKEN:
return None return None, "missing_slack_bot_token"
payload: dict[str, Any] = { payload: dict[str, Any] = {
"channel": channel_id, "channel": channel_id,
@ -350,21 +350,33 @@ async def post_slack_thread_reply_with_ts(
headers=_slack_headers(), headers=_slack_headers(),
json=payload, json=payload,
) )
if response.status_code == 429:
retry_after = response.headers.get("Retry-After")
logger.warning("Slack chat.postMessage rate limited (retry-after=%s)", retry_after)
if retry_after:
return None, f"rate_limited: {retry_after}"
return None, "rate_limited"
response.raise_for_status() response.raise_for_status()
data = response.json() data = response.json()
if not data.get("ok"): if not data.get("ok"):
logger.warning("Slack chat.postMessage failed: %s", data.get("error")) error = data.get("error")
return None logger.warning("Slack chat.postMessage failed: %s", error)
if error == "ratelimited":
return None, "rate_limited"
return None, error
message_ts = data.get("ts") message_ts = data.get("ts")
return message_ts if isinstance(message_ts, str) and message_ts else None if isinstance(message_ts, str) and message_ts:
except httpx.HTTPError: return message_ts, None
return None, None
except httpx.HTTPError as exc:
logger.exception("Slack chat.postMessage request failed") logger.exception("Slack chat.postMessage request failed")
return None return None, f"http_error: {type(exc).__name__}"
async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool: async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool:
"""Post a reply in a Slack thread.""" """Post a reply in a Slack thread."""
return await post_slack_thread_reply_with_ts(channel_id, thread_ts, text) is not None message_ts, _ = await post_slack_thread_reply_with_ts(channel_id, thread_ts, text)
return message_ts is not None
async def post_slack_ephemeral_message( async def post_slack_ephemeral_message(
@ -713,13 +725,14 @@ def _format_trace_reply(trace_url: str | None) -> str:
async def post_slack_trace_reply(channel_id: str, thread_ts: str, thread_id: str) -> str | None: async def post_slack_trace_reply(channel_id: str, thread_ts: str, thread_id: str) -> str | None:
"""Post a trace URL reply in a Slack thread and return its Slack timestamp.""" """Post a trace URL reply in a Slack thread and return its Slack timestamp."""
trace_url = get_langsmith_trace_url(thread_id) trace_url = get_langsmith_trace_url(thread_id)
return await post_slack_thread_reply_with_ts( message_ts, _ = await post_slack_thread_reply_with_ts(
channel_id, channel_id,
thread_ts, thread_ts,
_format_trace_reply(trace_url), _format_trace_reply(trace_url),
unfurl_links=False, unfurl_links=False,
unfurl_media=False, unfurl_media=False,
) )
return message_ts
_SLACK_RUN_MAP_NAMESPACE = "slack_run_map" _SLACK_RUN_MAP_NAMESPACE = "slack_run_map"

View file

@ -23,6 +23,13 @@ def _err_response(error: str = "channel_not_found") -> MagicMock:
return response return response
def _rate_limited_response(retry_after: str | None = None) -> MagicMock:
response = MagicMock()
response.status_code = 429
response.headers = {"Retry-After": retry_after} if retry_after else {}
return response
def _async_client_cm(post_response: MagicMock) -> AsyncMock: def _async_client_cm(post_response: MagicMock) -> AsyncMock:
client_cm = AsyncMock() client_cm = AsyncMock()
client_cm.__aenter__.return_value = client_cm client_cm.__aenter__.return_value = client_cm
@ -140,3 +147,96 @@ async def test_post_slack_thread_reply_does_not_call_set_status(
assert ok is True assert ok is True
assert client_cm.post.await_count == 1 assert client_cm.post.await_count == 1
assert client_cm.post.call_args.args[0].endswith("/chat.postMessage") assert client_cm.post.call_args.args[0].endswith("/chat.postMessage")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_returns_missing_token_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "")
client_cm = _async_client_cm(_ok_response())
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "missing_slack_bot_token")
client_cm.post.assert_not_called()
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_returns_slack_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_err_response("msg_too_long"))
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "msg_too_long")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_returns_rate_limited_with_retry_after(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_rate_limited_response(retry_after="30"))
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "rate_limited: 30")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_returns_rate_limited_without_retry_after(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_rate_limited_response())
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "rate_limited")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_normalizes_ratelimited_body_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_err_response("ratelimited"))
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "rate_limited")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_with_ts_returns_http_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_ok_response())
client_cm.post = AsyncMock(side_effect=slack_utils.httpx.ConnectError("boom"))
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
result = await slack_utils.post_slack_thread_reply_with_ts("C1", "1.0", "hello")
assert result == (None, "http_error: ConnectError")
@pytest.mark.asyncio
async def test_post_slack_thread_reply_preserves_bool_return_on_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setattr(slack_utils, "SLACK_BOT_TOKEN", "xoxb-test")
client_cm = _async_client_cm(_err_response("channel_not_found"))
with patch.object(slack_utils.httpx, "AsyncClient", return_value=client_cm):
ok = await slack_utils.post_slack_thread_reply("C1", "1.0", "hello")
assert ok is False

View file

@ -233,9 +233,9 @@ def test_post_slack_trace_reply_emits_tip_only_when_no_trace_url(
*, *,
unfurl_links: bool = True, unfurl_links: bool = True,
unfurl_media: bool = True, unfurl_media: bool = True,
) -> str | None: ) -> tuple[str | None, str | None]:
posted.append({"text": text, "unfurl_links": unfurl_links, "unfurl_media": unfurl_media}) posted.append({"text": text, "unfurl_links": unfurl_links, "unfurl_media": unfurl_media})
return "1.1" return "1.1", None
monkeypatch.setattr( monkeypatch.setattr(
slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts
@ -264,9 +264,9 @@ def test_post_slack_trace_reply_includes_trace_link_and_tip(
*, *,
unfurl_links: bool = True, unfurl_links: bool = True,
unfurl_media: bool = True, unfurl_media: bool = True,
) -> str | None: ) -> tuple[str | None, str | None]:
posted.append({"text": text, "unfurl_links": unfurl_links, "unfurl_media": unfurl_media}) posted.append({"text": text, "unfurl_links": unfurl_links, "unfurl_media": unfurl_media})
return "1.1" return "1.1", None
monkeypatch.setattr( monkeypatch.setattr(
slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts

View file

@ -0,0 +1,121 @@
from __future__ import annotations
import importlib
from typing import Any
import pytest
slack_reply_tool = importlib.import_module("agent.tools.slack_thread_reply")
def _config() -> dict[str, Any]:
return {
"configurable": {
"slack_thread": {
"channel_id": "C1",
"thread_ts": "1.0",
}
}
}
def test_slack_thread_reply_returns_structured_error_for_msg_too_long(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
return None, "msg_too_long"
monkeypatch.setattr(slack_reply_tool, "get_config", _config)
monkeypatch.setattr(slack_reply_tool, "_post_and_store_mapping", fake_post_and_store_mapping)
result = slack_reply_tool.slack_thread_reply("hello")
assert result == {
"success": False,
"error": "msg_too_long",
"slack_error": "msg_too_long",
"message_chars": 5,
"hint": "Slack rejected the message as too long; retry with a shorter message.",
}
@pytest.mark.parametrize("slack_error", ["channel_not_found", "not_in_channel"])
def test_slack_thread_reply_hints_not_to_retry_channel_errors(
slack_error: str,
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
return None, slack_error
monkeypatch.setattr(slack_reply_tool, "get_config", _config)
monkeypatch.setattr(slack_reply_tool, "_post_and_store_mapping", fake_post_and_store_mapping)
result = slack_reply_tool.slack_thread_reply("hello")
assert result["success"] is False
assert result["error"] == slack_error
assert result["slack_error"] == slack_error
assert result["message_chars"] == 5
assert "do not retry" in result["hint"]
assert "trace output" in result["hint"]
def test_slack_thread_reply_rate_limited_hint_includes_retry_after(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
return None, "rate_limited: 30"
monkeypatch.setattr(slack_reply_tool, "get_config", _config)
monkeypatch.setattr(slack_reply_tool, "_post_and_store_mapping", fake_post_and_store_mapping)
result = slack_reply_tool.slack_thread_reply("hello")
assert result["success"] is False
assert result["error"] == "rate_limited: 30"
assert result["slack_error"] == "rate_limited: 30"
assert "30s" in result["hint"]
assert "wait" in result["hint"]
def test_slack_thread_reply_rate_limited_hint_without_retry_after(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
return None, "rate_limited"
monkeypatch.setattr(slack_reply_tool, "get_config", _config)
monkeypatch.setattr(slack_reply_tool, "_post_and_store_mapping", fake_post_and_store_mapping)
result = slack_reply_tool.slack_thread_reply("hello")
assert result["success"] is False
assert result["slack_error"] == "rate_limited"
assert "wait" in result["hint"]
def test_slack_thread_reply_uses_post_failed_without_slack_error(
monkeypatch: pytest.MonkeyPatch,
) -> None:
async def fake_post_and_store_mapping(
channel_id: str, thread_ts: str, message: str
) -> tuple[str | None, str | None]:
return None, None
monkeypatch.setattr(slack_reply_tool, "get_config", _config)
monkeypatch.setattr(slack_reply_tool, "_post_and_store_mapping", fake_post_and_store_mapping)
result = slack_reply_tool.slack_thread_reply("hello")
assert result["success"] is False
assert result["error"] == "post failed"
assert result["slack_error"] is None
assert result["message_chars"] == 5