fix(gateway): log secondary startup-reconnect handoff failures instead of dropping them
Review follow-up to the AI code-review pass on PR #92074: the bridge task's handoff into _schedule_secondary_profile_reconnect was unguarded at both call sites inside the parked coroutine. The scheduler touches live registries (_profile_failed_platforms slot creation, background-task registration), so an unexpected raise there would kill the parked task as an unretrieved-task exception — logged only at GC time via "Task exception was never retrieved", where no operator ever looks. A fix whose entire purpose is to stop a platform dying silently should not contain its own silent-death path; both handoff sites now wrap the scheduler call with logger.exception so the failure lands in gateway.log with profile and platform context. The early-exit branch (gateway already _running when the bridge starts) had the identical exposure and is guarded the same way — same bug class, fixed together. Regression test drives a handoff raise end-to-end through the real bridge task: the await completes cleanly, the error is captured in gateway.run's logger, and no adapter or failed-platform slot leaks behind the failed handoff.
This commit is contained in:
+28
-6
@@ -15794,9 +15794,19 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
|
||||
async def _await_running_then_schedule() -> None:
|
||||
if self._running:
|
||||
self._schedule_secondary_profile_reconnect(
|
||||
profile_name, platform, adapter
|
||||
)
|
||||
try:
|
||||
self._schedule_secondary_profile_reconnect(
|
||||
profile_name, platform, adapter
|
||||
)
|
||||
except Exception:
|
||||
# Same GC-time-exception hazard as the post-poll handoff
|
||||
# below; surface it in gateway.log instead.
|
||||
logger.exception(
|
||||
"secondary-startup-reconnect handoff failed "
|
||||
"(profile=%s platform=%s)",
|
||||
profile_name,
|
||||
platform.value,
|
||||
)
|
||||
return
|
||||
# Modest poll interval: startup completion has no dedicated event,
|
||||
# and the reconnect runner's own backoff makes sub-100ms precision
|
||||
@@ -15804,9 +15814,21 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
||||
while not self._running and not self._shutdown_event.is_set():
|
||||
await asyncio.sleep(0.1)
|
||||
if self._running and not self._shutdown_event.is_set():
|
||||
self._schedule_secondary_profile_reconnect(
|
||||
profile_name, platform, adapter
|
||||
)
|
||||
try:
|
||||
self._schedule_secondary_profile_reconnect(
|
||||
profile_name, platform, adapter
|
||||
)
|
||||
except Exception:
|
||||
# The handoff touches live registries; if it raises, the
|
||||
# parked task would otherwise die as an unretrieved-task
|
||||
# exception logged only at GC time. Surface it where
|
||||
# operators look.
|
||||
logger.exception(
|
||||
"secondary-startup-reconnect handoff failed "
|
||||
"(profile=%s platform=%s)",
|
||||
profile_name,
|
||||
platform.value,
|
||||
)
|
||||
|
||||
task = asyncio.create_task(
|
||||
_await_running_then_schedule(),
|
||||
|
||||
@@ -420,6 +420,53 @@ class TestSecondaryStartupFailureRecovery:
|
||||
assert runner._background_tasks == set()
|
||||
assert runner._profile_failed_platforms == {}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handoff_failure_is_logged_not_raised(self, monkeypatch, caplog):
|
||||
"""If the scheduler raises at bridge handoff, the parked task must not
|
||||
die as an unretrieved-task exception — the failure surfaces in the log."""
|
||||
runner = _secondary_recovery_runner()
|
||||
failed = _SecondaryRecoveryAdapter()
|
||||
_install_secondary_reconnect_context(
|
||||
monkeypatch, runner, _SecondaryRecoveryAdapter()
|
||||
)
|
||||
monkeypatch.setattr(runner, "_create_adapter", lambda platform, config: failed)
|
||||
|
||||
async def fail_initial_connect(adapter, platform):
|
||||
return False
|
||||
|
||||
monkeypatch.setattr(
|
||||
runner, "_connect_initial_adapter_with_timeout", fail_initial_connect
|
||||
)
|
||||
|
||||
def explode_at_handoff(profile_name, platform, adapter):
|
||||
raise RuntimeError("scheduler exploded during handoff")
|
||||
|
||||
monkeypatch.setattr(
|
||||
runner, "_schedule_secondary_profile_reconnect", explode_at_handoff
|
||||
)
|
||||
|
||||
with caplog.at_level(logging.ERROR, logger="gateway.run"):
|
||||
connected = await runner._start_one_profile_adapters(
|
||||
"reviewer", "/tmp/reviewer", {}
|
||||
)
|
||||
bridge = list(runner._background_tasks)
|
||||
assert len(bridge) == 1
|
||||
# Awaiting completes cleanly: the guard swallows the handoff
|
||||
# failure instead of letting it escape as an unretrieved-task
|
||||
# exception at GC time.
|
||||
await asyncio.wait_for(bridge[0], timeout=0.5)
|
||||
|
||||
assert connected == 0
|
||||
assert failed.disconnected is True
|
||||
assert any(
|
||||
record.levelno == logging.ERROR
|
||||
and "secondary-startup-reconnect handoff failed" in record.getMessage()
|
||||
for record in caplog.records
|
||||
)
|
||||
# Nothing was scheduled and no slot leaked behind the failed handoff.
|
||||
assert Platform.DISCORD not in runner._profile_adapters.get("reviewer", {})
|
||||
assert runner._profile_failed_platforms == {}
|
||||
|
||||
|
||||
class TestSecondaryProfileConfigHandling:
|
||||
"""Secondary config errors degrade only when the profile is safe to skip."""
|
||||
|
||||
Reference in New Issue
Block a user