mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
Follow-up hardening for the salvaged #69240 readiness gate (#67498): - _start_polling_once now returns its (generation, progress_event) pair so the strict cold-start gate binds to exactly the generation it started, instead of re-reading self._polling_progress_event which a concurrent recovery task may have replaced with a newer generation's event (the G1/G2 race flagged in the #69240 review). - Strict cold start no longer schedules background polling recovery: a polling error during the readiness wait is captured by a strict callback and fails the connect attempt immediately with a loud OSError, so GatewayRunner disposes the partial adapter and retries with a fresh one — no more waiting out the full readiness deadline on a generation that already errored, and no G2-on-partial-app healing. - After readiness is proven the strict callback delegates every later polling error to the real background-recovery callback, preserving the existing degraded/reconnect semantics for the polling lifetime. - The readiness-timeout error message now states the deadline and that the gateway will retry with a fresh adapter (loud failure, not a silent wait). - Regression tests: current-generation progress connects; a polling error during strict cold start fails fast without scheduling background recovery (the #67498 idle-threads shape); stale-generation progress is rejected. Progresses #67498
279 lines
10 KiB
Python
279 lines
10 KiB
Python
"""Regression tests for #59614: start_polling() must be time-bounded.
|
|
|
|
When both the primary Telegram API server and all fallback IPs are unreachable,
|
|
``await app.updater.start_polling(...)`` can block forever inside an exhausted
|
|
httpx connection pool — it neither returns nor raises. Unbounded, that wedges:
|
|
|
|
1. the network-error reconnect ladder (stuck inside attempt 1, never advances),
|
|
2. the heartbeat loop (sees the recovery task as alive-but-wedged and skips),
|
|
3. the fatal-error escalation (never reached).
|
|
|
|
The fix wraps every ``start_polling()`` await in the wall-deadline helper with
|
|
``_UPDATER_START_TIMEOUT`` so a cancellation-shielded hung call still raises and
|
|
feeds the existing retry ladder. These tests patch the timeout down to keep the
|
|
suite fast.
|
|
"""
|
|
import asyncio
|
|
import sys
|
|
from unittest.mock import AsyncMock, MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
|
|
def _ensure_telegram_mock():
|
|
if "telegram" in sys.modules and hasattr(sys.modules["telegram"], "__file__"):
|
|
return
|
|
telegram_mod = MagicMock()
|
|
telegram_mod.ext.ContextTypes.DEFAULT_TYPE = type(None)
|
|
telegram_mod.constants.ParseMode.MARKDOWN_V2 = "MarkdownV2"
|
|
telegram_mod.constants.ChatType.GROUP = "group"
|
|
telegram_mod.constants.ChatType.SUPERGROUP = "supergroup"
|
|
telegram_mod.constants.ChatType.CHANNEL = "channel"
|
|
telegram_mod.constants.ChatType.PRIVATE = "private"
|
|
telegram_mod.error.NetworkError = type("NetworkError", (OSError,), {})
|
|
telegram_mod.error.TimedOut = type("TimedOut", (OSError,), {})
|
|
for name in ("telegram", "telegram.ext", "telegram.constants", "telegram.request"):
|
|
sys.modules.setdefault(name, telegram_mod)
|
|
sys.modules.setdefault("telegram.error", telegram_mod.error)
|
|
|
|
|
|
_ensure_telegram_mock()
|
|
|
|
from plugins.platforms.telegram import adapter as tg_adapter # noqa: E402
|
|
from plugins.platforms.telegram.adapter import TelegramAdapter # noqa: E402
|
|
|
|
|
|
async def _hang_forever(**kwargs):
|
|
await asyncio.sleep(1000)
|
|
|
|
|
|
def _bare_adapter():
|
|
a = TelegramAdapter.__new__(TelegramAdapter)
|
|
# `name` / `has_fatal_error` are read-only base-class properties; set the
|
|
# backing fields they derive from instead.
|
|
from gateway.config import Platform
|
|
|
|
a.platform = Platform.TELEGRAM
|
|
a._fatal_error_code = None
|
|
a._fatal_error_message = None
|
|
a._fatal_error_retryable = True
|
|
a._polling_network_error_count = 0
|
|
a._polling_conflict_count = 0
|
|
a._polling_error_callback_ref = None
|
|
a._background_tasks = set()
|
|
a._send_path_degraded = False
|
|
return a
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_network_ladder_start_polling_hang_does_not_wedge(monkeypatch):
|
|
"""A hung start_polling() in _handle_polling_network_error must time out
|
|
and advance the ladder instead of blocking forever (#59614 core repro)."""
|
|
monkeypatch.setattr(tg_adapter, "_UPDATER_START_TIMEOUT", 0.2)
|
|
a = _bare_adapter()
|
|
a._polling_network_error_count = 0 # attempt 1 → 5s backoff before start_polling
|
|
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
app.updater.start_polling = _hang_forever
|
|
app.updater.running = False
|
|
a._app = app
|
|
|
|
with patch.object(a, "_drain_polling_connections", new=AsyncMock()), \
|
|
patch.object(
|
|
tg_adapter.asyncio, "ensure_future",
|
|
side_effect=lambda coro: (coro.close(), asyncio.get_event_loop().create_future())[1],
|
|
):
|
|
# Unbounded, this await hangs past the 30s wait_for and fails the
|
|
# test; bounded, the handler waits its 5s backoff, times out the hung
|
|
# start_polling() in 0.2s, schedules the chained retry (captured by
|
|
# the ensure_future patch), and returns.
|
|
await asyncio.wait_for(
|
|
a._handle_polling_network_error(Exception("net down")), timeout=30
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_bootstrap_start_polling_hang_schedules_recovery(monkeypatch):
|
|
"""_start_polling_resilient: a hung bootstrap start_polling() must raise
|
|
TimeoutError (an OSError → classified as network error) and schedule
|
|
background recovery instead of blocking connect() forever."""
|
|
monkeypatch.setattr(tg_adapter, "_UPDATER_START_TIMEOUT", 0.2)
|
|
a = _bare_adapter()
|
|
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
app.updater.start_polling = _hang_forever
|
|
a._app = app
|
|
|
|
scheduled = []
|
|
monkeypatch.setattr(
|
|
a, "_schedule_polling_recovery",
|
|
lambda err, reason: scheduled.append((err, reason)),
|
|
raising=False,
|
|
)
|
|
|
|
ok = await asyncio.wait_for(
|
|
a._start_polling_resilient(drop_pending_updates=False, error_callback=None),
|
|
timeout=10,
|
|
)
|
|
assert ok is False
|
|
assert len(scheduled) == 1
|
|
assert isinstance(scheduled[0][0], (TimeoutError, asyncio.TimeoutError))
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_start_polling_success_path_unaffected(monkeypatch):
|
|
"""Sanity: a fast start_polling() still returns True through the wrapper."""
|
|
monkeypatch.setattr(tg_adapter, "_UPDATER_START_TIMEOUT", 5.0)
|
|
a = _bare_adapter()
|
|
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
app.updater.start_polling = AsyncMock(return_value=None)
|
|
a._app = app
|
|
|
|
ok = await a._start_polling_resilient(drop_pending_updates=False, error_callback=None)
|
|
assert ok is True
|
|
app.updater.start_polling.assert_awaited_once()
|
|
|
|
|
|
def test_every_start_polling_call_site_is_time_bounded():
|
|
"""Every updater.start_polling call must use the wall-deadline helper."""
|
|
import inspect
|
|
import re
|
|
|
|
src = inspect.getsource(tg_adapter)
|
|
lines = src.splitlines()
|
|
unbounded = []
|
|
for i, line in enumerate(lines):
|
|
if re.search(r"updater\.start_polling\(", line) and "def " not in line:
|
|
window = "\n".join(lines[max(0, i - 8):i + 1])
|
|
if "_await_with_thread_deadline" not in window:
|
|
unbounded.append((i + 1, line.strip()))
|
|
assert not unbounded, f"unbounded start_polling() call sites: {unbounded}"
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_initial_connect_requires_get_updates_progress(monkeypatch):
|
|
"""Cold connect must not claim success before real getUpdates I/O."""
|
|
monkeypatch.setattr(tg_adapter, "_INITIAL_POLLING_PROGRESS_TIMEOUT", 0.05)
|
|
a = _bare_adapter()
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
app.updater.start_polling = AsyncMock(return_value=None)
|
|
a._app = app
|
|
|
|
with pytest.raises(OSError, match="getUpdates made no progress"):
|
|
await a._start_polling_resilient(
|
|
drop_pending_updates=True,
|
|
error_callback=None,
|
|
require_progress=True,
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_initial_connect_succeeds_on_current_generation_progress(monkeypatch):
|
|
"""Strict cold start returns True once THIS generation records progress."""
|
|
monkeypatch.setattr(tg_adapter, "_INITIAL_POLLING_PROGRESS_TIMEOUT", 5.0)
|
|
a = _bare_adapter()
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
|
|
async def start_polling_with_progress(**_kwargs):
|
|
# PTB's instrumented getUpdates request records progress for the
|
|
# generation started by this call.
|
|
a._record_polling_progress(a._polling_generation)
|
|
|
|
app.updater.start_polling = AsyncMock(side_effect=start_polling_with_progress)
|
|
a._app = app
|
|
|
|
ok = await asyncio.wait_for(
|
|
a._start_polling_resilient(
|
|
drop_pending_updates=True,
|
|
error_callback=None,
|
|
require_progress=True,
|
|
),
|
|
timeout=10,
|
|
)
|
|
assert ok is True
|
|
assert a._send_path_degraded is False
|
|
# Strict mode owns readiness itself; the background verifier must not be
|
|
# racing it on the cold-start application.
|
|
assert getattr(a, "_polling_progress_verifier_task", None) is None
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_initial_connect_polling_error_fails_fast_not_background(monkeypatch):
|
|
"""#67498 idle-threads shape: a polling error during strict cold start
|
|
must surface immediately as a connect failure — NOT be swallowed into a
|
|
background recovery task that restarts polling on the partial app while
|
|
the readiness gate waits out its full deadline (the G1/G2 race from the
|
|
#69240 review)."""
|
|
monkeypatch.setattr(tg_adapter, "_INITIAL_POLLING_PROGRESS_TIMEOUT", 30.0)
|
|
a = _bare_adapter()
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
|
|
captured_callbacks = []
|
|
|
|
async def start_polling_capture(**kwargs):
|
|
captured_callbacks.append(kwargs.get("error_callback"))
|
|
|
|
app.updater.start_polling = AsyncMock(side_effect=start_polling_capture)
|
|
a._app = app
|
|
|
|
recovery_scheduled = []
|
|
a._schedule_polling_recovery = lambda err, reason: recovery_scheduled.append(err)
|
|
|
|
async def run_connect():
|
|
return await a._start_polling_resilient(
|
|
drop_pending_updates=True,
|
|
error_callback=lambda e: recovery_scheduled.append(e),
|
|
require_progress=True,
|
|
)
|
|
|
|
task = asyncio.ensure_future(run_connect())
|
|
# Let start_polling run and the strict gate begin waiting.
|
|
for _ in range(10):
|
|
await asyncio.sleep(0)
|
|
if captured_callbacks:
|
|
break
|
|
assert captured_callbacks and captured_callbacks[0] is not None
|
|
|
|
# PTB reports a network error on the first getUpdates (G1 error).
|
|
captured_callbacks[0](OSError("getUpdates failed"))
|
|
|
|
# The cold attempt must fail promptly (well before the 30s readiness
|
|
# deadline) with a loud OSError, and must NOT have scheduled background
|
|
# recovery (which would start G2 on the same partial application).
|
|
with pytest.raises(OSError, match="errored before first getUpdates"):
|
|
await asyncio.wait_for(task, timeout=5)
|
|
assert recovery_scheduled == []
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_initial_connect_ignores_stale_generation_progress(monkeypatch):
|
|
"""Progress recorded for a stale generation must not satisfy readiness."""
|
|
monkeypatch.setattr(tg_adapter, "_INITIAL_POLLING_PROGRESS_TIMEOUT", 0.1)
|
|
a = _bare_adapter()
|
|
app = MagicMock()
|
|
app.updater = AsyncMock()
|
|
|
|
async def start_polling_stale_progress(**_kwargs):
|
|
# Progress arrives tagged with a PREVIOUS generation (e.g. a late
|
|
# response from an abandoned attempt) — must be rejected.
|
|
a._record_polling_progress(a._polling_generation - 1)
|
|
|
|
app.updater.start_polling = AsyncMock(side_effect=start_polling_stale_progress)
|
|
a._app = app
|
|
|
|
with pytest.raises(OSError, match="getUpdates made no progress"):
|
|
await asyncio.wait_for(
|
|
a._start_polling_resilient(
|
|
drop_pending_updates=True,
|
|
error_callback=None,
|
|
require_progress=True,
|
|
),
|
|
timeout=10,
|
|
)
|