From dce4abe9172d6478bed73525efdd49e6d10bed03 Mon Sep 17 00:00:00 2001 From: ruangraung Date: Sun, 23 Aug 2026 04:01:39 +0700 Subject: [PATCH] fix(gateway): log secondary startup-reconnect handoff failures instead of dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- gateway/run.py | 34 +++++++++++--- .../test_multiplex_adapter_registry.py | 47 +++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 6f1cba96e3..3175ecdeed 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -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(), diff --git a/tests/gateway/test_multiplex_adapter_registry.py b/tests/gateway/test_multiplex_adapter_registry.py index 8a0799873c..d048d9a0ea 100644 --- a/tests/gateway/test_multiplex_adapter_registry.py +++ b/tests/gateway/test_multiplex_adapter_registry.py @@ -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."""