mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-09 21:49:37 +00:00
fix(discord): prevent typing tasks after stop (#4752)
* fix(discord): prevent typing tasks after stop * fix(discord): serialize typing cleanup on event loop * fix(discord): harden typing cleanup on loop exit
This commit is contained in:
parent
46fd5c8a00
commit
df01102dfc
File diff suppressed because one or more lines are too long
@ -136,6 +136,8 @@ FastAPI application providing REST endpoints for frontend integration:
|
||||
|
||||
The IM bridge supports Feishu, Slack, and Telegram. Slack and Telegram still use the final `runs.wait()` response path, while Feishu now streams through `runs.stream(["messages-tuple", "values"])`, serializes rapid same-thread turns inside the channel manager, and updates a single in-thread card per source message in place.
|
||||
|
||||
Discord registers each typing-indicator loop before inbound message handling yields and refuses to start new typing work after the channel stops. Typing tasks are owned by the dedicated Discord event loop, so normal shutdown schedules bounded cancellation, awaiting, and map cleanup on that loop before closing the client. The Discord worker also drains the tasks in its `finally` block while its loop is still usable, covering disconnect and exception exits; if `stop()` encounters an already-stopped foreign loop, it never awaits those loop-bound tasks from the main loop. This serializes registration and cleanup across the main and Discord threads while preventing shutdown hangs and cross-loop `RuntimeError`s.
|
||||
|
||||
For Feishu card updates, DeerFlow stores the running card's `message_id` per inbound message and patches that same card until the run finishes, preserving the existing `OK` / `DONE` reaction flow. When a follow-up arrives inside an existing Feishu topic while another turn is still running, the later message now waits on the mapped DeerFlow `thread_id`, receives a queued/running card on that exact source message, and keeps a compact source-message blockquote in subsequent patches so rapid consecutive questions remain distinguishable.
|
||||
|
||||
---
|
||||
|
||||
@ -174,26 +174,56 @@ class DiscordChannel(Channel):
|
||||
self._running = False
|
||||
self.bus.unsubscribe_outbound(self._on_outbound)
|
||||
|
||||
# Cancel all active typing indicator tasks
|
||||
for target_id, task in list(self._typing_tasks.items()):
|
||||
if not task.done():
|
||||
task.cancel()
|
||||
logger.debug("[Discord] cancelled typing task for target %s", target_id)
|
||||
self._typing_tasks.clear()
|
||||
|
||||
if self._client and self._discord_loop and self._discord_loop.is_running():
|
||||
close_future = asyncio.run_coroutine_threadsafe(self._client.close(), self._discord_loop)
|
||||
discord_loop = self._discord_loop
|
||||
current_loop = asyncio.get_running_loop()
|
||||
if discord_loop is None or discord_loop is current_loop:
|
||||
await self._cancel_typing_tasks()
|
||||
elif discord_loop.is_running():
|
||||
# Serialize cleanup with _start_typing() on the owning loop. The
|
||||
# loop may exit between is_running() and scheduling, so neither
|
||||
# scheduling nor waiting is allowed to block the rest of stop().
|
||||
cleanup_coro = self._cancel_typing_tasks()
|
||||
try:
|
||||
await asyncio.wait_for(asyncio.wrap_future(close_future), timeout=10)
|
||||
except TimeoutError:
|
||||
logger.warning("[Discord] client close timed out after 10s")
|
||||
except Exception:
|
||||
logger.exception("[Discord] error while closing client")
|
||||
cleanup_future = asyncio.run_coroutine_threadsafe(cleanup_coro, discord_loop)
|
||||
except RuntimeError:
|
||||
cleanup_coro.close()
|
||||
logger.warning("[Discord] event loop stopped before typing-task cleanup could be scheduled")
|
||||
else:
|
||||
try:
|
||||
await asyncio.wait_for(asyncio.wrap_future(cleanup_future), timeout=10)
|
||||
except TimeoutError:
|
||||
cleanup_future.cancel()
|
||||
logger.warning("[Discord] typing-task cleanup timed out after 10s")
|
||||
except Exception:
|
||||
logger.exception("[Discord] error while cleaning up typing tasks")
|
||||
|
||||
if self._client and discord_loop and discord_loop.is_running():
|
||||
close_coro = self._client.close()
|
||||
try:
|
||||
close_future = asyncio.run_coroutine_threadsafe(close_coro, discord_loop)
|
||||
except RuntimeError:
|
||||
close_coro.close()
|
||||
logger.warning("[Discord] event loop stopped before client close could be scheduled")
|
||||
else:
|
||||
try:
|
||||
await asyncio.wait_for(asyncio.wrap_future(close_future), timeout=10)
|
||||
except TimeoutError:
|
||||
close_future.cancel()
|
||||
logger.warning("[Discord] client close timed out after 10s")
|
||||
except Exception:
|
||||
logger.exception("[Discord] error while closing client")
|
||||
|
||||
if self._thread:
|
||||
self._thread.join(timeout=10)
|
||||
self._thread = None
|
||||
|
||||
# _run_client() normally drains these tasks in its finally block. If
|
||||
# the owning loop was stopped externally before that cleanup ran, only
|
||||
# discard the stale references here; awaiting them from this loop would
|
||||
# raise a cross-loop RuntimeError.
|
||||
if discord_loop and discord_loop is not current_loop and not discord_loop.is_running():
|
||||
self._discard_typing_tasks()
|
||||
|
||||
self._client = None
|
||||
self._discord_loop = None
|
||||
self._discord_module = None
|
||||
@ -244,6 +274,8 @@ class DiscordChannel(Channel):
|
||||
|
||||
async def _start_typing(self, channel, chat_id: str, thread_ts: str | None = None) -> None:
|
||||
"""Starts a loop to send periodic typing indicators."""
|
||||
if not self._running:
|
||||
return
|
||||
target_id = thread_ts or chat_id
|
||||
if target_id in self._typing_tasks:
|
||||
return # Already typing for this target
|
||||
@ -262,6 +294,27 @@ class DiscordChannel(Channel):
|
||||
task = asyncio.create_task(_typing_loop())
|
||||
self._typing_tasks[target_id] = task
|
||||
|
||||
async def _cancel_typing_tasks(self) -> None:
|
||||
"""Cancel and await every typing task on their owning event loop."""
|
||||
typing_tasks = list(self._typing_tasks.items())
|
||||
for target_id, task in typing_tasks:
|
||||
if not task.done():
|
||||
task.cancel()
|
||||
logger.debug("[Discord] cancelled typing task for target %s", target_id)
|
||||
if typing_tasks:
|
||||
await asyncio.gather(*(task for _, task in typing_tasks), return_exceptions=True)
|
||||
self._typing_tasks.clear()
|
||||
|
||||
def _discard_typing_tasks(self) -> None:
|
||||
"""Forget stale typing tasks after their owning event loop has stopped."""
|
||||
for target_id, task in self._typing_tasks.items():
|
||||
if not task.done():
|
||||
logger.warning(
|
||||
"[Discord] discarding pending typing task for stopped-loop target %s",
|
||||
target_id,
|
||||
)
|
||||
self._typing_tasks.clear()
|
||||
|
||||
async def _stop_typing(self, chat_id: str, thread_ts: str | None = None) -> None:
|
||||
"""Stops the typing loop for a specific target."""
|
||||
target_id = thread_ts or chat_id
|
||||
@ -352,7 +405,7 @@ class DiscordChannel(Channel):
|
||||
self._publish(inbound)
|
||||
# Start typing indicator in the thread
|
||||
if typing_target:
|
||||
asyncio.create_task(self._start_typing(typing_target, chat_id, thread_id))
|
||||
await self._start_typing(typing_target, chat_id, thread_id)
|
||||
asyncio.create_task(self._add_reaction(message))
|
||||
return
|
||||
|
||||
@ -460,7 +513,7 @@ class DiscordChannel(Channel):
|
||||
|
||||
# Start typing indicator in the correct target (thread or channel)
|
||||
if typing_target:
|
||||
asyncio.create_task(self._start_typing(typing_target, chat_id, thread_id))
|
||||
await self._start_typing(typing_target, chat_id, thread_id)
|
||||
|
||||
self._publish(inbound)
|
||||
asyncio.create_task(self._add_reaction(message))
|
||||
@ -534,6 +587,10 @@ class DiscordChannel(Channel):
|
||||
if self._running:
|
||||
logger.exception("Discord client error")
|
||||
finally:
|
||||
try:
|
||||
self._discord_loop.run_until_complete(self._cancel_typing_tasks())
|
||||
except Exception:
|
||||
logger.exception("Error while cleaning up Discord typing tasks")
|
||||
try:
|
||||
if self._client and not self._client.is_closed():
|
||||
self._discord_loop.run_until_complete(self._client.close())
|
||||
|
||||
@ -5692,6 +5692,219 @@ class TestHandleChatWithArtifacts:
|
||||
_run(go())
|
||||
|
||||
|
||||
class TestDiscordChannel:
|
||||
def test_stop_prevents_queued_typing_starter_from_creating_task(self):
|
||||
from app.channels.discord import DiscordChannel
|
||||
|
||||
async def go():
|
||||
channel = DiscordChannel(MessageBus(), config={})
|
||||
channel._running = True
|
||||
typing_target = SimpleNamespace(trigger_typing=AsyncMock())
|
||||
|
||||
# Queue the starter without yielding to it. stop() therefore runs
|
||||
# first and must form a boundary that the delayed starter cannot
|
||||
# cross by installing a fresh infinite typing loop afterwards.
|
||||
starter = asyncio.create_task(channel._start_typing(typing_target, "chat-1"))
|
||||
await channel.stop()
|
||||
await starter
|
||||
|
||||
try:
|
||||
assert channel._typing_tasks == {}
|
||||
finally:
|
||||
leaked_tasks = list(channel._typing_tasks.values())
|
||||
for task in leaked_tasks:
|
||||
task.cancel()
|
||||
await asyncio.gather(*leaked_tasks, return_exceptions=True)
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_stop_serializes_typing_cleanup_with_discord_loop(self):
|
||||
from app.channels.discord import DiscordChannel
|
||||
|
||||
class BlockingTypingTasks(dict):
|
||||
def __init__(self, lookup_started: threading.Event, release_lookup: threading.Event):
|
||||
super().__init__()
|
||||
self._lookup_started = lookup_started
|
||||
self._release_lookup = release_lookup
|
||||
self.registered_task = None
|
||||
|
||||
def __contains__(self, key):
|
||||
self._lookup_started.set()
|
||||
if not self._release_lookup.wait(timeout=5):
|
||||
raise TimeoutError("typing-task lookup was not released")
|
||||
return super().__contains__(key)
|
||||
|
||||
def __setitem__(self, key, value):
|
||||
self.registered_task = value
|
||||
super().__setitem__(key, value)
|
||||
|
||||
async def go():
|
||||
channel = DiscordChannel(MessageBus(), config={})
|
||||
channel._running = True
|
||||
typing_target = SimpleNamespace(trigger_typing=AsyncMock())
|
||||
|
||||
discord_loop = asyncio.new_event_loop()
|
||||
loop_ready = threading.Event()
|
||||
|
||||
def run_discord_loop():
|
||||
asyncio.set_event_loop(discord_loop)
|
||||
loop_ready.set()
|
||||
discord_loop.run_forever()
|
||||
|
||||
discord_thread = threading.Thread(target=run_discord_loop, daemon=True)
|
||||
discord_thread.start()
|
||||
assert await asyncio.to_thread(loop_ready.wait, 5)
|
||||
|
||||
lookup_started = threading.Event()
|
||||
release_lookup = threading.Event()
|
||||
stop_started = threading.Event()
|
||||
channel._discord_loop = discord_loop
|
||||
typing_tasks = BlockingTypingTasks(lookup_started, release_lookup)
|
||||
channel._typing_tasks = typing_tasks
|
||||
channel.bus.unsubscribe_outbound = MagicMock(side_effect=lambda _callback: stop_started.set())
|
||||
|
||||
starter = asyncio.run_coroutine_threadsafe(channel._start_typing(typing_target, "chat-1"), discord_loop)
|
||||
stop_task = None
|
||||
|
||||
try:
|
||||
# Pause the Discord loop after _start_typing() has observed
|
||||
# _running=True but before it can create and register the task.
|
||||
assert await asyncio.to_thread(lookup_started.wait, 5)
|
||||
|
||||
stop_task = asyncio.create_task(channel.stop())
|
||||
# unsubscribe_outbound() runs immediately after stop() flips
|
||||
# _running to False. Once this fires, release the Discord
|
||||
# loop so any queued cleanup can serialize after registration.
|
||||
assert await asyncio.to_thread(stop_started.wait, 5)
|
||||
release_lookup.set()
|
||||
|
||||
await asyncio.wait_for(stop_task, timeout=5)
|
||||
await asyncio.wait_for(asyncio.wrap_future(starter), timeout=5)
|
||||
|
||||
assert typing_tasks == {}
|
||||
assert typing_tasks.registered_task is not None
|
||||
assert typing_tasks.registered_task.done()
|
||||
finally:
|
||||
release_lookup.set()
|
||||
if stop_task is not None and not stop_task.done():
|
||||
stop_task.cancel()
|
||||
await asyncio.gather(stop_task, return_exceptions=True)
|
||||
|
||||
if not starter.done():
|
||||
starter.cancel()
|
||||
await asyncio.gather(asyncio.wrap_future(starter), return_exceptions=True)
|
||||
|
||||
async def cleanup_typing_tasks():
|
||||
leaked_tasks = list(channel._typing_tasks.values())
|
||||
for task in leaked_tasks:
|
||||
task.cancel()
|
||||
await asyncio.gather(*leaked_tasks, return_exceptions=True)
|
||||
channel._typing_tasks.clear()
|
||||
|
||||
cleanup = asyncio.run_coroutine_threadsafe(cleanup_typing_tasks(), discord_loop)
|
||||
await asyncio.wait_for(asyncio.wrap_future(cleanup), timeout=5)
|
||||
discord_loop.call_soon_threadsafe(discord_loop.stop)
|
||||
await asyncio.to_thread(discord_thread.join, 5)
|
||||
assert not discord_thread.is_alive()
|
||||
discord_loop.close()
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_stop_does_not_await_typing_tasks_from_stopped_discord_loop(self):
|
||||
from app.channels.discord import DiscordChannel
|
||||
|
||||
async def go():
|
||||
channel = DiscordChannel(MessageBus(), config={})
|
||||
channel._running = True
|
||||
typing_target = SimpleNamespace(trigger_typing=AsyncMock())
|
||||
|
||||
discord_loop = asyncio.new_event_loop()
|
||||
loop_ready = threading.Event()
|
||||
|
||||
def run_discord_loop():
|
||||
asyncio.set_event_loop(discord_loop)
|
||||
loop_ready.set()
|
||||
discord_loop.run_forever()
|
||||
|
||||
discord_thread = threading.Thread(target=run_discord_loop, daemon=True)
|
||||
discord_thread.start()
|
||||
assert await asyncio.to_thread(loop_ready.wait, 5)
|
||||
|
||||
channel._discord_loop = discord_loop
|
||||
channel._thread = discord_thread
|
||||
starter = asyncio.run_coroutine_threadsafe(channel._start_typing(typing_target, "chat-1"), discord_loop)
|
||||
await asyncio.wait_for(asyncio.wrap_future(starter), timeout=5)
|
||||
typing_task = channel._typing_tasks["chat-1"]
|
||||
|
||||
discord_loop.call_soon_threadsafe(discord_loop.stop)
|
||||
await asyncio.to_thread(discord_thread.join, 5)
|
||||
assert not discord_thread.is_alive()
|
||||
assert not discord_loop.is_running()
|
||||
assert not typing_task.done()
|
||||
|
||||
try:
|
||||
# The task belongs to a loop that has already exited. stop()
|
||||
# must not gather it from the main loop, and must still finish
|
||||
# clearing all channel lifecycle state.
|
||||
await channel.stop()
|
||||
|
||||
assert channel._typing_tasks == {}
|
||||
assert channel._thread is None
|
||||
assert channel._discord_loop is None
|
||||
finally:
|
||||
|
||||
def drain_stopped_loop():
|
||||
asyncio.set_event_loop(discord_loop)
|
||||
if not typing_task.done():
|
||||
typing_task.cancel()
|
||||
discord_loop.run_until_complete(asyncio.gather(typing_task, return_exceptions=True))
|
||||
discord_loop.close()
|
||||
|
||||
await asyncio.to_thread(drain_stopped_loop)
|
||||
|
||||
_run(go())
|
||||
|
||||
def test_run_client_drains_typing_tasks_before_worker_loop_exits(self):
|
||||
from app.channels.discord import DiscordChannel
|
||||
|
||||
channel = DiscordChannel(MessageBus(), config={})
|
||||
channel._running = True
|
||||
typing_target = SimpleNamespace(trigger_typing=AsyncMock())
|
||||
|
||||
class FailingClient:
|
||||
def __init__(self):
|
||||
self.closed = False
|
||||
self.typing_task = None
|
||||
|
||||
async def start(self, _token):
|
||||
await channel._start_typing(typing_target, "chat-1")
|
||||
self.typing_task = channel._typing_tasks["chat-1"]
|
||||
raise RuntimeError("simulated disconnect")
|
||||
|
||||
def is_closed(self):
|
||||
return self.closed
|
||||
|
||||
async def close(self):
|
||||
self.closed = True
|
||||
|
||||
client = FailingClient()
|
||||
channel._client = client
|
||||
channel._bot_token = "token"
|
||||
|
||||
try:
|
||||
with patch("app.channels.discord.logger.exception"):
|
||||
channel._run_client()
|
||||
|
||||
assert channel._typing_tasks == {}
|
||||
assert client.typing_task is not None
|
||||
assert client.typing_task.done()
|
||||
assert client.closed
|
||||
finally:
|
||||
discord_loop = channel._discord_loop
|
||||
if discord_loop is not None and not discord_loop.is_closed():
|
||||
discord_loop.close()
|
||||
|
||||
|
||||
class TestFeishuChannel:
|
||||
def test_prepare_inbound_publishes_without_waiting_for_running_card(self):
|
||||
from app.channels.feishu import FeishuChannel
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user