fix(discord): auto-thread failure must not silently fall back to inline reply
When discord.auto_thread is enabled and a top-level server-channel message should be routed to a new thread, a transient thread-create failure (e.g. Cannot connect to host discord.com:443) returned None and _handle_message fell through to an inline parent-channel reply — dumping a new task into a shared channel and breaking thread-first workflows. - _auto_create_thread retries the primary + seed-message paths once after a 750ms backoff for transient connect errors. - _handle_message treats None as a hard failure: posts a short visible notice in the parent channel and returns without invoking the agent. The notify send is wrapped so a secondary connect error can't raise. Fixes #20243
This commit is contained in:
0xsir0000
Teknium
parent
a537baa81d
commit
50a7dce6bd
3 changed files with 144 additions and 21 deletions
|
|
@ -5020,7 +5020,11 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
async def _auto_create_thread(self, message: 'DiscordMessage') -> Optional[Any]:
|
||||
"""Create a thread from a user message for auto-threading.
|
||||
|
||||
Returns the created thread object, or ``None`` on failure.
|
||||
Returns the created thread object, or ``None`` on failure. Both the
|
||||
primary ``message.create_thread`` and the seed-message fallback are
|
||||
retried once after a short backoff so transient connect errors
|
||||
(e.g. ``Cannot connect to host discord.com:443``) don't immediately
|
||||
burn through to the caller's failure path (#20243).
|
||||
"""
|
||||
# Build a short thread name from the message. Strip Discord mention
|
||||
# syntax (users / roles / channels) so thread titles don't end up
|
||||
|
|
@ -5035,28 +5039,44 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
if len(content) > 80:
|
||||
thread_name = thread_name[:77] + "..."
|
||||
|
||||
try:
|
||||
thread = await message.create_thread(name=thread_name, auto_archive_duration=1440)
|
||||
return thread
|
||||
except Exception as direct_error:
|
||||
display_name = getattr(getattr(message, "author", None), "display_name", None) or "unknown user"
|
||||
reason = f"Auto-threaded from mention by {display_name}"
|
||||
display_name = getattr(getattr(message, "author", None), "display_name", None) or "unknown user"
|
||||
reason = f"Auto-threaded from mention by {display_name}"
|
||||
|
||||
last_direct_error: Exception | None = None
|
||||
last_fallback_error: Exception | None = None
|
||||
|
||||
for attempt in range(2):
|
||||
try:
|
||||
seed_msg = await message.channel.send(f"\U0001f9f5 Thread created by Hermes: **{thread_name}**")
|
||||
thread = await seed_msg.create_thread(
|
||||
name=thread_name,
|
||||
auto_archive_duration=1440,
|
||||
reason=reason,
|
||||
)
|
||||
thread = await message.create_thread(name=thread_name, auto_archive_duration=1440)
|
||||
return thread
|
||||
except Exception as fallback_error:
|
||||
logger.warning(
|
||||
"[%s] Auto-thread creation failed. Direct error: %s. Fallback error: %s",
|
||||
self.name,
|
||||
direct_error,
|
||||
fallback_error,
|
||||
)
|
||||
return None
|
||||
except Exception as direct_error:
|
||||
last_direct_error = direct_error
|
||||
try:
|
||||
seed_msg = await message.channel.send(
|
||||
f"\U0001f9f5 Thread created by Hermes: **{thread_name}**"
|
||||
)
|
||||
thread = await seed_msg.create_thread(
|
||||
name=thread_name,
|
||||
auto_archive_duration=1440,
|
||||
reason=reason,
|
||||
)
|
||||
return thread
|
||||
except Exception as fallback_error:
|
||||
last_fallback_error = fallback_error
|
||||
if attempt == 0:
|
||||
# Brief backoff before the second attempt — most failures
|
||||
# in this path are transient connect errors that recover
|
||||
# within a second or two.
|
||||
await asyncio.sleep(0.75)
|
||||
continue
|
||||
|
||||
logger.warning(
|
||||
"[%s] Auto-thread creation failed after retry. Direct error: %s. Fallback error: %s",
|
||||
self.name,
|
||||
last_direct_error,
|
||||
last_fallback_error,
|
||||
)
|
||||
return None
|
||||
|
||||
async def create_handoff_thread(
|
||||
self,
|
||||
|
|
@ -5742,6 +5762,26 @@ class DiscordAdapter(BasePlatformAdapter):
|
|||
# event is dropped before it can trigger a second agent run.
|
||||
# Fixes #51057.
|
||||
self._dedup.is_duplicate(str(thread.id))
|
||||
else:
|
||||
# Auto-threading is the configured routing target for this
|
||||
# message; if it fails we must NOT silently fall back to an
|
||||
# inline parent-channel reply (#20243). That breaks
|
||||
# thread-first Discord workflows by dumping a new task into
|
||||
# a shared channel. Surface a short visible error so the
|
||||
# user can retry once Discord recovers, and skip agent
|
||||
# invocation for this message.
|
||||
try:
|
||||
await message.channel.send(
|
||||
"⚠️ Hermes could not create a Discord thread for "
|
||||
"this message, so the request was not processed. Please retry."
|
||||
)
|
||||
except Exception as notify_error:
|
||||
logger.warning(
|
||||
"[%s] Failed to notify user of auto-thread failure: %s",
|
||||
self.name,
|
||||
notify_error,
|
||||
)
|
||||
return
|
||||
|
||||
referenced_attachments = []
|
||||
reference = getattr(message, "reference", None)
|
||||
|
|
|
|||
|
|
@ -140,6 +140,11 @@ async def test_non_ignored_channel_processes_normally(adapter, monkeypatch):
|
|||
monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "500,600")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
|
||||
# Stub auto-thread creation so this test focuses on ignored-channel
|
||||
# routing only — auto-thread failures now correctly skip agent invocation
|
||||
# (#20243), which would otherwise mask the assertion below.
|
||||
adapter._auto_create_thread = AsyncMock(return_value=FakeThread(channel_id=999))
|
||||
|
||||
message = make_message(channel=FakeTextChannel(channel_id=700), content="hello")
|
||||
await adapter._handle_message(message)
|
||||
|
||||
|
|
@ -167,6 +172,11 @@ async def test_ignored_channels_empty_string_ignores_nothing(adapter, monkeypatc
|
|||
monkeypatch.setenv("DISCORD_IGNORED_CHANNELS", "")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
|
||||
# Stub auto-thread creation so this test focuses on ignored-channel
|
||||
# routing only — auto-thread failures now correctly skip agent invocation
|
||||
# (#20243), which would otherwise mask the assertion below.
|
||||
adapter._auto_create_thread = AsyncMock(return_value=FakeThread(channel_id=999))
|
||||
|
||||
message = make_message(channel=FakeTextChannel(channel_id=500), content="hello")
|
||||
await adapter._handle_message(message)
|
||||
|
||||
|
|
@ -281,6 +291,71 @@ async def test_no_thread_with_auto_thread_disabled_is_noop(adapter, monkeypatch)
|
|||
adapter.handle_message.assert_awaited_once()
|
||||
|
||||
|
||||
# ── auto-thread failure must not silently fall back to inline (#20243) ──
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_thread_failure_skips_agent_and_notifies_user(adapter, monkeypatch):
|
||||
"""Auto-thread creation failure must not trigger an inline parent-channel reply.
|
||||
|
||||
Before #20243, ``effective_channel = auto_threaded_channel or message.channel``
|
||||
silently routed the response back to the parent channel when thread creation
|
||||
failed, breaking thread-first Discord workflows. The fix surfaces a short
|
||||
visible error to the parent channel and skips agent invocation entirely so
|
||||
the user can retry.
|
||||
"""
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false")
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "true")
|
||||
monkeypatch.delenv("DISCORD_NO_THREAD_CHANNELS", raising=False)
|
||||
monkeypatch.delenv("DISCORD_IGNORED_CHANNELS", raising=False)
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
|
||||
adapter._auto_create_thread = AsyncMock(return_value=None)
|
||||
|
||||
channel = FakeTextChannel(channel_id=800)
|
||||
channel.send = AsyncMock()
|
||||
message = make_message(channel=channel, content="hello")
|
||||
await adapter._handle_message(message)
|
||||
|
||||
adapter._auto_create_thread.assert_awaited_once()
|
||||
# Agent must NOT be invoked when the routing target failed.
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
# User gets a visible explanation in the parent channel instead of a silent
|
||||
# inline reply.
|
||||
channel.send.assert_awaited_once()
|
||||
sent_text = channel.send.await_args.args[0]
|
||||
assert "could not create" in sent_text.lower()
|
||||
assert "thread" in sent_text.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_thread_failure_notify_error_does_not_crash(adapter, monkeypatch):
|
||||
"""If even the failure-notification send raises, we still skip the agent.
|
||||
|
||||
``message.channel.send`` itself can fail (the same connect issue that
|
||||
killed thread creation often kills plain sends too). The handler should
|
||||
swallow the secondary error and still avoid invoking the agent.
|
||||
"""
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false")
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "true")
|
||||
monkeypatch.delenv("DISCORD_NO_THREAD_CHANNELS", raising=False)
|
||||
monkeypatch.delenv("DISCORD_IGNORED_CHANNELS", raising=False)
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
|
||||
adapter._auto_create_thread = AsyncMock(return_value=None)
|
||||
|
||||
channel = FakeTextChannel(channel_id=800)
|
||||
channel.send = AsyncMock(side_effect=RuntimeError("Cannot connect to host discord.com:443"))
|
||||
message = make_message(channel=channel, content="hello")
|
||||
|
||||
# No exception must propagate.
|
||||
await adapter._handle_message(message)
|
||||
|
||||
adapter._auto_create_thread.assert_awaited_once()
|
||||
adapter.handle_message.assert_not_awaited()
|
||||
channel.send.assert_awaited_once()
|
||||
|
||||
|
||||
# ── config.py bridging ───────────────────────────────────────────────
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -202,6 +202,10 @@ async def test_discord_defaults_to_require_mention(adapter, monkeypatch):
|
|||
async def test_discord_free_response_in_server_channels(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "false")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
# Auto-thread failures now correctly skip agent invocation (#20243), and
|
||||
# FakeTextChannel has no real ``create_thread``. Disable auto-thread so the
|
||||
# routing assertion below stays focused on free-response gating.
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "false")
|
||||
|
||||
message = make_message(channel=FakeTextChannel(channel_id=123), content="hello from channel")
|
||||
|
||||
|
|
@ -334,6 +338,10 @@ async def test_discord_forum_parent_in_free_response_list_allows_forum_thread(ad
|
|||
async def test_discord_accepts_and_strips_bot_mentions_when_required(adapter, monkeypatch):
|
||||
monkeypatch.setenv("DISCORD_REQUIRE_MENTION", "true")
|
||||
monkeypatch.delenv("DISCORD_FREE_RESPONSE_CHANNELS", raising=False)
|
||||
# Auto-thread failures now correctly skip agent invocation (#20243).
|
||||
# FakeTextChannel can't satisfy the real ``create_thread`` API, so disable
|
||||
# auto-thread to keep this test focused on mention-strip behaviour.
|
||||
monkeypatch.setenv("DISCORD_AUTO_THREAD", "false")
|
||||
|
||||
bot_user = adapter._client.user
|
||||
message = make_message(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue