fix(discord): clear fatal status on successful reconnect
DiscordAdapter.connect() set self._running = True directly instead of
calling self._mark_connected(), unlike every other platform adapter
(Telegram, WeCom, Matrix, Feishu, Google Chat, IRC, LINE, Mattermost,
ntfy, photon, raft, simplex, a2a, buzz, dingtalk, whatsapp).
_mark_connected() clears _fatal_error_code/_fatal_error_message/
_fatal_error_retryable and rewrites the runtime status file as
"connected". Bypassing it meant a transient connect failure (e.g. a
one-off DNS blip: "Cannot connect to host discord.com:443 ssl:default
[Temporary failure in name resolution]") left the platform reported as
permanently fatal in gateway_state.json / the dashboard, even after the
adapter successfully reconnected and was actively serving messages for
hours.
Reproduced under gateway.multiplex_profiles: true with a secondary
profile's Discord bot (sarathi:discord) — the bot reconnected
repeatedly ("Connected as ..." logged many times over 18+ hours) while
the dashboard kept showing the original fatal error the entire time.
Fixes #102554.
Added a regression test asserting connect() clears a previously
recorded fatal error.
This commit is contained in:
@@ -1352,7 +1352,9 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter):
|
||||
self._ready_event, self._bot_task,
|
||||
timeout=None if ready_timeout <= 0 else ready_timeout,
|
||||
)
|
||||
self._running = True
|
||||
# _mark_connected() clears a prior fatal stamp; a bare ``_running = True`` left a transient
|
||||
# startup failure reported as ``fatal`` for the life of the process (#102554).
|
||||
self._mark_connected()
|
||||
self._start_liveness_probe()
|
||||
# Plugin-registered native handlers (discord.py Bot — add_listener()/event hooks).
|
||||
self._wire_plugin_handlers(self._client)
|
||||
|
||||
@@ -246,6 +246,51 @@ async def test_reconnect_closes_previous_client_to_prevent_zombie_websocket(monk
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_clears_previous_fatal_error(monkeypatch):
|
||||
"""Regression: a successful connect() must clear any prior fatal-error
|
||||
state via _mark_connected(), not just set self._running = True directly.
|
||||
|
||||
Every other platform adapter (Telegram, WeCom, Matrix, ...) calls
|
||||
_mark_connected() on a successful connect, which clears
|
||||
_fatal_error_code/_fatal_error_message/_fatal_error_retryable and
|
||||
rewrites the runtime status file as "connected". DiscordAdapter used to
|
||||
bypass this and set self._running = True directly, so a transient
|
||||
connect failure (e.g. a DNS blip) left the platform reported as
|
||||
permanently "fatal" in the dashboard/gateway_state.json even after the
|
||||
adapter reconnected successfully and was actively serving messages.
|
||||
"""
|
||||
adapter = DiscordAdapter(PlatformConfig(enabled=True, token="test-token"))
|
||||
|
||||
monkeypatch.setattr("gateway.status.acquire_scoped_lock", lambda scope, identity, metadata=None: (True, None))
|
||||
monkeypatch.setattr("gateway.status.release_scoped_lock", lambda scope, identity: None)
|
||||
|
||||
intents = SimpleNamespace(
|
||||
message_content=False, dm_messages=False, guild_messages=False,
|
||||
members=False, voice_states=False,
|
||||
)
|
||||
monkeypatch.setattr(discord_platform.Intents, "default", lambda: intents)
|
||||
monkeypatch.setattr(discord_platform.commands, "Bot", FakeBot)
|
||||
monkeypatch.setattr(adapter, "_resolve_allowed_usernames", AsyncMock())
|
||||
|
||||
# Simulate a prior fatal error left over from a transient connect failure.
|
||||
adapter._set_fatal_error("discord_connect_error", "Discord startup failed: boom", retryable=True)
|
||||
assert adapter.has_fatal_error is True
|
||||
|
||||
assert await adapter.connect() is True
|
||||
|
||||
assert adapter.has_fatal_error is False, (
|
||||
"connect() must clear a previously recorded fatal error via "
|
||||
"_mark_connected() so the dashboard doesn't show a stale failure "
|
||||
"forever after a successful reconnect"
|
||||
)
|
||||
assert adapter.fatal_error_code is None
|
||||
assert adapter.fatal_error_message is None
|
||||
assert adapter._running is True
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_connect_timeout_cancels_bot_task(monkeypatch):
|
||||
"""Regression: connect() timeout must cancel _bot_task so the zombie
|
||||
|
||||
Reference in New Issue
Block a user