test(discord): bind event-silence liveness to its config surface and probe gate
The event-silence tests duplicated `_make_adapter`/`_connect` from test_discord_liveness.py and carried a bespoke handler-wait loop. Reuse the sibling helpers instead: `_make_adapter` grows an optional `max_event_silence` that is only written into `extra` when given, so the sibling's own tests keep the adapter default and stay unchanged. Two invariants now bind the fix: - deaf socket: a `None` stamp (no DISPATCH yet) reads healthy across several probe intervals, and once armed, transport-green + event silence trips `event_silence` through `_liveness_loop`. - `websocket_event_max_silence_seconds: 0` disables only that dimension: with a stale stamp and a stale heartbeat ACK the probe must still run and trip on `ack_stale`. This goes red if the knob is moved into `_start_liveness_probe`'s all-or-nothing guard (#109782). The YAML->extra passthrough test also asserts the new key, so dropping its `_YAML_WEBSOCKET_LIVENESS_KEYS` entry fails.
This commit is contained in:
@@ -360,7 +360,8 @@ class TestLoadGatewayConfig:
|
||||
" websocket_liveness_interval_seconds: 17\n"
|
||||
" websocket_liveness_failure_threshold: 4\n"
|
||||
" websocket_heartbeat_ack_max_age_seconds: 75\n"
|
||||
" websocket_max_latency_seconds: 30\n",
|
||||
" websocket_max_latency_seconds: 30\n"
|
||||
" websocket_event_max_silence_seconds: 7200\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
@@ -377,6 +378,7 @@ class TestLoadGatewayConfig:
|
||||
assert extra["websocket_liveness_failure_threshold"] == 4
|
||||
assert extra["websocket_heartbeat_ack_max_age_seconds"] == 75
|
||||
assert extra["websocket_max_latency_seconds"] == 30
|
||||
assert extra["websocket_event_max_silence_seconds"] == 7200
|
||||
|
||||
|
||||
def test_quick_commands_from_nested_gateway_section(self, tmp_path, monkeypatch):
|
||||
|
||||
@@ -11,25 +11,21 @@ unlike the debug-gated ``on_socket_raw_receive``) and trips after
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
import time
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
|
||||
from tests.gateway.test_discord_connect import ( # noqa: E402
|
||||
FakeBot,
|
||||
_ensure_discord_mock,
|
||||
)
|
||||
from tests.gateway.test_discord_connect import _ensure_discord_mock # noqa: E402
|
||||
|
||||
_ensure_discord_mock()
|
||||
|
||||
import plugins.platforms.discord.adapter as discord_platform # noqa: E402
|
||||
from gateway.config import PlatformConfig # noqa: E402
|
||||
from plugins.platforms.discord.adapter import DiscordAdapter # noqa: E402
|
||||
|
||||
from tests.gateway.test_discord_liveness import ( # noqa: E402
|
||||
_LiveBot,
|
||||
_connect,
|
||||
_make_adapter,
|
||||
_set_websocket_health,
|
||||
_wait_until,
|
||||
)
|
||||
|
||||
|
||||
@@ -53,127 +49,72 @@ class _DispatchingBot(_LiveBot):
|
||||
await handler(event_type)
|
||||
|
||||
|
||||
def _make_adapter(
|
||||
monkeypatch,
|
||||
*,
|
||||
interval: float = 0.01,
|
||||
threshold: int = 1,
|
||||
max_ack_age: float = 60.0,
|
||||
max_latency: float = 30.0,
|
||||
max_event_silence: float = 14400.0,
|
||||
) -> DiscordAdapter:
|
||||
monkeypatch.setenv("HERMES_DISCORD_LIVENESS_INTERVAL_SECONDS", str(interval))
|
||||
monkeypatch.setenv("HERMES_DISCORD_LIVENESS_FAILURE_THRESHOLD", str(threshold))
|
||||
return DiscordAdapter(
|
||||
PlatformConfig(
|
||||
enabled=True,
|
||||
token="test-token",
|
||||
extra={
|
||||
"websocket_heartbeat_ack_max_age_seconds": max_ack_age,
|
||||
"websocket_max_latency_seconds": max_latency,
|
||||
"websocket_event_max_silence_seconds": max_event_silence,
|
||||
},
|
||||
)
|
||||
)
|
||||
def _dispatching_bot(**kwargs) -> _DispatchingBot:
|
||||
bot = _DispatchingBot(intents=kwargs["intents"], allowed_mentions=kwargs.get("allowed_mentions"))
|
||||
bot.fetch_user = AsyncMock()
|
||||
return bot
|
||||
|
||||
|
||||
async def _connect(adapter: DiscordAdapter, monkeypatch, bot_factory) -> FakeBot:
|
||||
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", bot_factory)
|
||||
monkeypatch.setattr(adapter, "_resolve_allowed_usernames", AsyncMock())
|
||||
assert await adapter.connect() is True
|
||||
return adapter._client
|
||||
|
||||
|
||||
def _transport_healthy(bot: _DispatchingBot) -> None:
|
||||
def _transport_healthy(bot: _DispatchingBot, *, ack_age: float = 0.0) -> None:
|
||||
"""Make every transport-side sample read healthy (incident 2's fingerprint)."""
|
||||
_set_websocket_health(bot, ready=True, socket_open=True, latency=0.05, ack_age=0.0)
|
||||
_set_websocket_health(bot, ready=True, socket_open=True, latency=0.05, ack_age=ack_age)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deaf_socket_trips_event_silence_dimension(monkeypatch):
|
||||
"""Incident 2 e2e: transport-green + event-starved must trip the probe.
|
||||
|
||||
The probe samples through ``_liveness_loop``, the real dispatch surface
|
||||
(no direct ``_read_websocket_health`` call), so this also proves the
|
||||
dimension is gated inside the health check and not in the startup guard.
|
||||
Sampled through ``_liveness_loop`` (the real dispatch surface). Before the
|
||||
first DISPATCH the stamp is ``None`` — "nothing parsed yet", owned by
|
||||
``not_ready`` — and must not read as silence, or fresh reconnects on quiet
|
||||
guilds would false-trip.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch, interval=0.01, threshold=2, max_event_silence=0.05)
|
||||
handler = AsyncMock()
|
||||
adapter.set_fatal_error_handler(handler)
|
||||
|
||||
def factory(**kwargs):
|
||||
bot = _DispatchingBot(intents=kwargs["intents"], allowed_mentions=kwargs.get("allowed_mentions"))
|
||||
bot.fetch_user = AsyncMock()
|
||||
return bot
|
||||
|
||||
bot = await _connect(adapter, monkeypatch, factory)
|
||||
await _connect(adapter, monkeypatch, _dispatching_bot)
|
||||
bot = adapter._client
|
||||
_transport_healthy(bot)
|
||||
assert adapter._last_dispatched_event_monotonic is None
|
||||
|
||||
# Several probe intervals past the silence bound with no stamp: still healthy.
|
||||
await asyncio.sleep(0.2)
|
||||
assert adapter._fatal_error_code is None
|
||||
assert adapter._read_websocket_health(bot) == (True, "healthy")
|
||||
|
||||
# One early event arms the stamp; then total DISPATCH silence while every
|
||||
# transport sample stays green.
|
||||
await bot.deliver_dispatch("READY")
|
||||
|
||||
async def handler_awaited() -> None:
|
||||
# _liveness_loop sets the fatal code, then hands off to
|
||||
# _notify_liveness_fatal_error (a separate task that closes the client
|
||||
# — up to a 1s budget — before notifying the runner), so wait for the
|
||||
# handler itself, not just the code.
|
||||
while True:
|
||||
if handler.await_count:
|
||||
return
|
||||
code = getattr(adapter, "_fatal_error_code", None)
|
||||
if code and adapter._liveness_notification_task is not None:
|
||||
with_context = adapter._liveness_notification_task
|
||||
try:
|
||||
await asyncio.wait_for(asyncio.shield(with_context), timeout=3.0)
|
||||
except (asyncio.TimeoutError, asyncio.CancelledError, Exception):
|
||||
pass
|
||||
if handler.await_count:
|
||||
return
|
||||
await asyncio.sleep(0.01)
|
||||
|
||||
await asyncio.wait_for(handler_awaited(), timeout=8.0)
|
||||
# _liveness_loop sets the code, then a separate task closes the client (1s
|
||||
# budget) before notifying the runner — so wait for the handler itself.
|
||||
await _wait_until(lambda: handler.await_count, "fatal handler never awaited", timeout=8.0)
|
||||
assert adapter._fatal_error_code == "discord_websocket_health_stale"
|
||||
assert "event_silence" in (adapter._fatal_error_message or "")
|
||||
assert adapter._fatal_error_retryable is True
|
||||
handler.assert_awaited_once()
|
||||
|
||||
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_missing_stamp_before_first_event_is_not_silence(monkeypatch):
|
||||
"""A connected client with no DISPATCH event yet must not read as deaf.
|
||||
async def test_zero_silence_bound_disables_only_that_dimension(monkeypatch):
|
||||
"""``websocket_event_max_silence_seconds: 0`` must not switch the watchdog off.
|
||||
|
||||
``None`` means "nothing parsed on this connection" — the pre-READY
|
||||
window; ``not_ready`` owns that failure shape. Treating ``None`` as
|
||||
silence would false-trip fresh reconnects on quiet guilds.
|
||||
The #109782 regression put the knob in ``_start_liveness_probe``'s
|
||||
all-or-nothing guard, so opting out of event-silence also dropped the
|
||||
ack-age/latency guards. With a stale stamp AND a stale heartbeat ACK the
|
||||
probe must still run and trip on ``ack_stale``.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch, interval=0.01, threshold=1, max_event_silence=0.05)
|
||||
adapter = _make_adapter(monkeypatch, interval=0.01, threshold=1, max_ack_age=60.0, max_event_silence=0)
|
||||
handler = AsyncMock()
|
||||
adapter.set_fatal_error_handler(handler)
|
||||
|
||||
def factory(**kwargs):
|
||||
bot = _DispatchingBot(intents=kwargs["intents"], allowed_mentions=kwargs.get("allowed_mentions"))
|
||||
bot.fetch_user = AsyncMock()
|
||||
return bot
|
||||
await _connect(adapter, monkeypatch, _dispatching_bot)
|
||||
bot = adapter._client
|
||||
_transport_healthy(bot, ack_age=120.0)
|
||||
adapter._last_dispatched_event_monotonic = time.perf_counter() - 1000.0
|
||||
|
||||
bot = await _connect(adapter, monkeypatch, factory)
|
||||
_transport_healthy(bot)
|
||||
assert adapter._last_dispatched_event_monotonic is None
|
||||
|
||||
deadline = asyncio.get_running_loop().time() + 0.4
|
||||
while asyncio.get_running_loop().time() < deadline:
|
||||
healthy, reason = adapter._read_websocket_health(bot)
|
||||
assert healthy is True, f"None-stamp window must read healthy, got {reason}"
|
||||
await asyncio.sleep(0.05)
|
||||
|
||||
await adapter.disconnect()
|
||||
await _wait_until(lambda: handler.await_count, "probe did not run with the knob at 0", timeout=8.0)
|
||||
assert adapter._fatal_error_code == "discord_websocket_health_stale"
|
||||
assert "ack_stale" in (adapter._fatal_error_message or "")
|
||||
assert "event_silence" not in (adapter._fatal_error_message or "")
|
||||
|
||||
@@ -95,19 +95,18 @@ def _make_adapter(
|
||||
threshold=1,
|
||||
max_ack_age=1.0,
|
||||
max_latency=1.0,
|
||||
max_event_silence: float | None = None,
|
||||
) -> DiscordAdapter:
|
||||
monkeypatch.setenv("HERMES_DISCORD_LIVENESS_INTERVAL_SECONDS", str(interval))
|
||||
monkeypatch.setenv("HERMES_DISCORD_LIVENESS_FAILURE_THRESHOLD", str(threshold))
|
||||
return DiscordAdapter(
|
||||
PlatformConfig(
|
||||
enabled=True,
|
||||
token="test-token",
|
||||
extra={
|
||||
"websocket_heartbeat_ack_max_age_seconds": max_ack_age,
|
||||
"websocket_max_latency_seconds": max_latency,
|
||||
},
|
||||
)
|
||||
)
|
||||
extra = {
|
||||
"websocket_heartbeat_ack_max_age_seconds": max_ack_age,
|
||||
"websocket_max_latency_seconds": max_latency,
|
||||
}
|
||||
# Only the event-silence tests pin this knob; everyone else keeps the adapter default.
|
||||
if max_event_silence is not None:
|
||||
extra["websocket_event_max_silence_seconds"] = max_event_silence
|
||||
return DiscordAdapter(PlatformConfig(enabled=True, token="test-token", extra=extra))
|
||||
|
||||
|
||||
class _BrokenWebSocket:
|
||||
|
||||
Reference in New Issue
Block a user