mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 17:23:15 +00:00
fix: surface Slack thread errors (#1627)
* fix: surface Slack thread errors
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
* fix: don't set failure_reply_posted on Slack preprocessing errors
The preprocessing error handler was setting failure_reply_posted=True,
the same idempotency flag handle_run_completion checks to suppress
duplicate run-failure replies. Since preprocessing failures happen
before any run exists but the flag persists on the thread, a subsequent
run failure on the same thread would be silently ignored.
The preprocessing handler already posts its own Slack reply, so the
run-completion idempotency flag should not be set here.
---------
Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
(cherry picked from commit bb36448b0b)
This commit is contained in:
parent
3c6077c418
commit
e8b6fb7050
4 changed files with 162 additions and 3 deletions
|
|
@ -19,6 +19,7 @@ import os
|
|||
from collections.abc import Awaitable, Callable
|
||||
from typing import Any
|
||||
|
||||
from .utils.dashboard_links import dashboard_thread_url
|
||||
from .utils.github_app import get_github_app_installation_token
|
||||
from .utils.github_comments import post_github_comment
|
||||
from .utils.linear import comment_on_linear_issue
|
||||
|
|
@ -63,12 +64,15 @@ def verify_run_complete_token(token: str | None) -> bool:
|
|||
return token is not None and hmac.compare_digest(token, secret)
|
||||
|
||||
|
||||
def _failure_text(status: str) -> str:
|
||||
def _failure_text(status: str, dashboard_url: str | None = None) -> str:
|
||||
reason = "timed out" if status == "timeout" else "hit an unexpected error"
|
||||
return (
|
||||
text = (
|
||||
f"⚠️ I wasn't able to finish that — the run {reason}. "
|
||||
"Send another message and I'll pick it back up."
|
||||
)
|
||||
if dashboard_url:
|
||||
text += f" You can view the error in <{dashboard_url}|Open SWE Web>."
|
||||
return text
|
||||
|
||||
|
||||
async def _post_failure_reply(
|
||||
|
|
@ -96,7 +100,8 @@ async def _post_failure_reply(
|
|||
thread_ts = slack_thread.get("thread_ts")
|
||||
if channel_id and thread_ts:
|
||||
await claim()
|
||||
return await post_slack_thread_reply(channel_id, thread_ts, text)
|
||||
slack_text = _failure_text(status, dashboard_thread_url(thread_id))
|
||||
return await post_slack_thread_reply(channel_id, thread_ts, slack_text)
|
||||
return False
|
||||
|
||||
if source == "linear":
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ Helpers and constants stay in webapp.py; they are accessed through the module
|
|||
object (``webapp.X``) so tests that monkeypatch them keep working.
|
||||
"""
|
||||
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
|
|
@ -43,6 +44,84 @@ def _format_slack_thread_section(
|
|||
|
||||
async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[str, str]) -> None:
|
||||
"""Process a Slack app mention by creating a run or queuing a mid-run message."""
|
||||
try:
|
||||
await _process_slack_mention_impl(event_data, repo_config)
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.exception("Unexpected error while processing Slack mention")
|
||||
await _notify_slack_processing_error(event_data, repo_config)
|
||||
|
||||
|
||||
async def _notify_slack_processing_error(
|
||||
event_data: dict[str, Any], repo_config: dict[str, str]
|
||||
) -> None:
|
||||
channel_id = event_data.get("channel_id", "")
|
||||
thread_ts = event_data.get("thread_ts", "")
|
||||
event_ts = event_data.get("event_ts", "")
|
||||
user_id = event_data.get("user_id", "")
|
||||
text = event_data.get("text", "")
|
||||
bot_user_id = event_data.get("bot_user_id", "")
|
||||
if not channel_id or not thread_ts:
|
||||
return
|
||||
|
||||
thread_id = webapp.generate_thread_id_from_slack_thread(channel_id, thread_ts)
|
||||
try:
|
||||
clean_text = (
|
||||
webapp.strip_bot_mention(text, bot_user_id, bot_username=webapp.SLACK_BOT_USERNAME)
|
||||
or "Slack request"
|
||||
)
|
||||
await webapp.upsert_agent_thread_owner_metadata(
|
||||
thread_id,
|
||||
source="slack",
|
||||
repo_config=repo_config,
|
||||
title=clean_text,
|
||||
source_context={
|
||||
"slack_thread": {
|
||||
"channel_id": channel_id,
|
||||
"thread_ts": thread_ts,
|
||||
"triggering_user_id": user_id,
|
||||
"triggering_event_ts": event_ts,
|
||||
}
|
||||
},
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.warning(
|
||||
"Could not persist Slack error metadata for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
|
||||
try:
|
||||
await webapp.get_client(url=webapp.LANGGRAPH_URL).threads.update(
|
||||
thread_id=thread_id,
|
||||
metadata={
|
||||
"latest_run_status": "error",
|
||||
"updated_at_ms": int(datetime.now(UTC).timestamp() * 1000),
|
||||
},
|
||||
)
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.warning("Could not mark Slack thread %s as errored", thread_id, exc_info=True)
|
||||
|
||||
try:
|
||||
await webapp.set_slack_assistant_status(channel_id, thread_ts, status="")
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.debug("Could not clear Slack assistant status", exc_info=True)
|
||||
|
||||
dashboard_url = webapp.dashboard_thread_url(thread_id)
|
||||
message = (
|
||||
"⚠️ I hit an unexpected error while handling this Slack thread. "
|
||||
"Send another message and I'll try again."
|
||||
)
|
||||
if dashboard_url:
|
||||
message += f" You can view the error in <{dashboard_url}|Open SWE Web>."
|
||||
try:
|
||||
await webapp.post_slack_thread_reply(channel_id, thread_ts, message)
|
||||
except Exception: # noqa: BLE001
|
||||
webapp.logger.warning(
|
||||
"Could not post Slack error notification for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
|
||||
|
||||
async def _process_slack_mention_impl(
|
||||
event_data: dict[str, Any], repo_config: dict[str, str]
|
||||
) -> None:
|
||||
channel_id = event_data.get("channel_id", "")
|
||||
thread_ts = event_data.get("thread_ts", "")
|
||||
event_ts = event_data.get("event_ts", "")
|
||||
|
|
|
|||
|
|
@ -38,6 +38,9 @@ async def test_error_status_posts_slack_failure_reply(monkeypatch: pytest.Monkey
|
|||
monkeypatch.setattr(completion, "langgraph_client", lambda: client)
|
||||
reply = AsyncMock(return_value=True)
|
||||
monkeypatch.setattr(completion, "post_slack_thread_reply", reply)
|
||||
monkeypatch.setattr(
|
||||
completion, "dashboard_thread_url", lambda thread_id: f"https://ui/{thread_id}"
|
||||
)
|
||||
|
||||
result = await completion.handle_run_completion({"thread_id": "t1", "status": "error"})
|
||||
|
||||
|
|
@ -46,6 +49,7 @@ async def test_error_status_posts_slack_failure_reply(monkeypatch: pytest.Monkey
|
|||
args = reply.await_args.args
|
||||
assert args[0] == "C1"
|
||||
assert args[1] == "123.45"
|
||||
assert "<https://ui/t1|Open SWE Web>" in args[2]
|
||||
assert client.threads.updates == [{"failure_reply_posted": True}]
|
||||
|
||||
|
||||
|
|
|
|||
71
tests/test_slack_webhook_errors.py
Normal file
71
tests/test_slack_webhook_errors.py
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
from typing import Any
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.webhooks import slack as slack_webhook
|
||||
|
||||
|
||||
class _FakeThreads:
|
||||
def __init__(self) -> None:
|
||||
self.updates: list[dict[str, Any]] = []
|
||||
|
||||
async def update(self, *, thread_id: str, metadata: dict[str, Any]) -> None:
|
||||
self.updates.append({"thread_id": thread_id, "metadata": metadata})
|
||||
|
||||
|
||||
class _FakeClient:
|
||||
def __init__(self) -> None:
|
||||
self.threads = _FakeThreads()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_slack_processing_error_posts_dashboard_link(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
async def fail_processing(event_data: dict[str, Any], repo_config: dict[str, str]) -> None:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
client = _FakeClient()
|
||||
upsert = AsyncMock()
|
||||
set_status = AsyncMock()
|
||||
post_reply = AsyncMock(return_value=True)
|
||||
|
||||
monkeypatch.setattr(slack_webhook, "_process_slack_mention_impl", fail_processing)
|
||||
monkeypatch.setattr(
|
||||
slack_webhook.webapp, "generate_thread_id_from_slack_thread", lambda *_: "t1"
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
slack_webhook.webapp, "strip_bot_mention", lambda text, *_args, **_kwargs: text
|
||||
)
|
||||
monkeypatch.setattr(slack_webhook.webapp, "upsert_agent_thread_owner_metadata", upsert)
|
||||
monkeypatch.setattr(slack_webhook.webapp, "get_client", lambda *, url: client)
|
||||
monkeypatch.setattr(slack_webhook.webapp, "set_slack_assistant_status", set_status)
|
||||
monkeypatch.setattr(
|
||||
slack_webhook.webapp, "dashboard_thread_url", lambda thread_id: f"https://ui/{thread_id}"
|
||||
)
|
||||
monkeypatch.setattr(slack_webhook.webapp, "post_slack_thread_reply", post_reply)
|
||||
|
||||
await slack_webhook.process_slack_mention(
|
||||
{
|
||||
"channel_id": "C1",
|
||||
"thread_ts": "123.45",
|
||||
"event_ts": "123.45",
|
||||
"user_id": "U1",
|
||||
"text": "help",
|
||||
"bot_user_id": "BOT",
|
||||
},
|
||||
{"owner": "langchain-ai", "name": "open-swe"},
|
||||
)
|
||||
|
||||
upsert.assert_awaited_once()
|
||||
assert len(client.threads.updates) == 1
|
||||
update = client.threads.updates[0]
|
||||
assert update["thread_id"] == "t1"
|
||||
assert update["metadata"]["latest_run_status"] == "error"
|
||||
assert "failure_reply_posted" not in update["metadata"]
|
||||
assert isinstance(update["metadata"]["updated_at_ms"], int)
|
||||
set_status.assert_awaited_once_with("C1", "123.45", status="")
|
||||
post_reply.assert_awaited_once()
|
||||
assert post_reply.await_args.args[:2] == ("C1", "123.45")
|
||||
assert "<https://ui/t1|Open SWE Web>" in post_reply.await_args.args[2]
|
||||
Loading…
Add table
Reference in a new issue