Skip to content

fix(gateway): respawn dead reconnect watcher + faulthandler for event-loop freeze diagnosis - #71177

Closed
kshitijk4poor wants to merge 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/70502-reconnect-watcher-respawn
Closed

fix(gateway): respawn dead reconnect watcher + faulthandler for event-loop freeze diagnosis#71177
kshitijk4poor wants to merge 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/70502-reconnect-watcher-respawn

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Ensures the platform reconnect watcher is always alive after queueing a retryable fatal adapter error, and adds faulthandler at gateway startup for post-mortem diagnosis of event-loop freezes.

Root cause (#70344): if the reconnect watcher task dies (exhausts its _spawn_supervised restart budget), platforms queued for reconnection are permanently stranded — the gateway stays alive but never rebuilds deaf adapters.

Changes

  • gateway/run.py: _ensure_reconnect_watcher_running() detects a dead reconnect watcher task and respawns it via _spawn_supervised (preserving crash-restart supervision). Called from _handle_adapter_fatal_error_impl after queueing a retryable fatal error.
  • gateway/run.py: faulthandler.enable() + SIGUSR2 dump to gateway_faulthandler.log for post-mortem diagnosis of event-loop freezes.
  • tests/gateway/test_platform_reconnect.py: 8 new tests (4 for _ensure_reconnect_watcher_running, 2 for fatal error integration, 2 for detach-on-timeout)

Salvage fixes on top of contributor's commit:

  • _ensure_reconnect_watcher_running uses _spawn_supervised instead of bare asyncio.create_task (preserves backoff, crash-loop detection, healthy-run reset)
  • Faulthandler log path uses _hermes_home (module-level get_hermes_home()) instead of os.environ.get("HERMES_HOME") (respects profile overrides)
  • Faulthandler file handle stored in self._faulthandler_file for clean shutdown (no leaked file handle)
  • _connect_adapter_with_timeout uses task.result() instead of bool(result) (preserves original return value)

Credit: @webtecnica's original commit cherry-picked with authorship preserved.

Validation

Before After
Reconnect watcher dies platforms permanently stranded respawned via _spawn_supervised
Event-loop freeze no diagnosis SIGUSR2 dumps all thread stacks
Targeted tests 46 passed

Related: #70345 (detach-on-timeout, same issue's defense-in-depth concern)

Infographic

Image generation is unavailable in this environment (no FAL_KEY / portal credits). The infographic will be attached once available.

webtecnica and others added 2 commits July 25, 2026 08:28
…esearch#70344)

Cherry-picked from @webtecnica's PR NousResearch#70502. The reconnect watcher
task can die after exhausting its _spawn_supervised restart budget,
leaving platforms permanently stranded. _ensure_reconnect_watcher_running
detects a dead task and respawns it.
…th + file handle

- _ensure_reconnect_watcher_running now uses _spawn_supervised instead of
  bare asyncio.create_task, preserving crash-restart supervision (backoff,
  crash-loop detection, healthy-run reset)
- Faulthandler log path uses _hermes_home (already available at module level)
  instead of os.environ.get('HERMES_HOME'), respecting profile overrides
- Faulthandler file handle stored in self._faulthandler_file for clean shutdown
- _connect_adapter_with_timeout: use task.result() instead of bool(result)
  to preserve the original return value from adapter.connect()
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) July 25, 2026 03:29
@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #70344 and focused sibling #71176. This broader salvage includes the detach-on-timeout repair plus watcher-respawn recovery; the overlapping patches need a maintainer consolidation choice.

@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded. The detach-on-timeout + _ensure_reconnect_watcher_running + faulthandler work landed on main via #70987, and the one remaining live delta (spawning the reconnect watcher via _spawn_supervised) merged via #72366 (salvage of #71867) — with better test coverage and a fix for the double-watcher handle-tracking gap. Thanks for the salvage attempt.

@teknium1 teknium1 closed this Jul 27, 2026
auto-merge was automatically disabled July 27, 2026 02:32

Pull request was closed

@kshitijk4poor
kshitijk4poor deleted the salvage/70502-reconnect-watcher-respawn branch August 5, 2026 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P1 High — major feature broken, no workaround sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants