mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(dashboard): disable ws keepalive ping on loopback to survive event-loop stalls
Desktop/dashboard WebSocket connections drop during long agent operations (delegate_task subagents, large model outputs) when the uvicorn event loop is GIL-starved for minutes. Root cause: uvicorn's ws keepalive ping runs on the SAME event loop as agent turns. A single synchronous GIL-holding call on a worker thread (a regex/scrub over a large output, or a long subagent turn) freezes the loop, so it cannot process the incoming pong within ws_ping_timeout and uvicorn closes an otherwise-healthy connection (#53773: 'event loop stalled 226.3s'; #48445/#50005). Loosening the timeout only raises the threshold — a multi-minute stall sails past any finite window. The keepalive ping exists to detect half-open connections (reverse-proxy 524, dropped tunnels), which cannot happen on loopback: there is no network or proxy in the path, and a dead local client tears the socket down with a real FIN/RST that starlette surfaces as WebSocketDisconnect regardless of the ping. So on loopback the ping provides ~no liveness value while actively killing recoverable stalls — disable it entirely (ws_ping_interval/timeout=None). Non-loopback (public) binds sit behind a Cloudflare Tunnel where half-open IS a real failure mode, so the ping stays at 20/20 to detect it. Empirically verified (real uvicorn + websockets peer): with ws_ping=None the server never closes a silent peer during an 8s window; with the pre-fix 2s/2s window uvicorn closes it. A genuinely-dead client still fires the WebSocketDisconnect reap path regardless of the ping. Note: this fixes the local Desktop case (the OP's scenario). A remote Desktop over an authenticated public dashboard route (McCalebTheSecond's comment) keeps the ping and needs the deeper GIL-hotspot fix — tracked separately. Closes #53773
This commit is contained in:
parent
65cb70b8d0
commit
e73adb5043
2 changed files with 60 additions and 21 deletions
|
|
@ -14119,13 +14119,24 @@ def start_server(
|
|||
# OSError inside create_server() and exits with a clear error — no
|
||||
# separate preflight probe needed.
|
||||
# Loopback binds are the Desktop case: a single local client, no reverse
|
||||
# proxy in front. A GIL-heavy agent turn can stall the event loop past 20s,
|
||||
# and uvicorn's ws keepalive ping runs on that same starved loop — so a
|
||||
# 20s ping timeout kills an otherwise-healthy local connection over a
|
||||
# recoverable stall (QW-1). Give loopback a longer 60s timeout / 30s
|
||||
# interval to ride out those stalls. Non-loopback binds sit behind a
|
||||
# Cloudflare Tunnel (idle timeout ~100s), so keep them at 20/20 to detect
|
||||
# half-open connections promptly and stay under the tunnel's idle window.
|
||||
# proxy in front. uvicorn's ws keepalive ping runs ON the same event loop
|
||||
# as agent turns, and a single synchronous GIL-holding call on a worker
|
||||
# thread (e.g. a regex/scrub over a large model output, or a long
|
||||
# delegate_task subagent turn) can starve that loop for *minutes* — the
|
||||
# loop cannot process the incoming pong, so uvicorn declares the socket
|
||||
# dead and closes it, dropping an otherwise-healthy local connection
|
||||
# (#53773: "event loop stalled 226.3s"; #48445/#50005). A longer timeout
|
||||
# only raises the threshold — a multi-minute stall sails past any finite
|
||||
# window. The keepalive ping exists to detect *half-open* connections
|
||||
# (reverse-proxy 524, dropped tunnels), which cannot happen on loopback:
|
||||
# there is no network or proxy in the path, and a dead local client tears
|
||||
# the socket down with a real FIN/RST that starlette surfaces as
|
||||
# WebSocketDisconnect regardless of the ping. So on loopback the ping
|
||||
# provides ~no liveness value while actively killing recoverable stalls —
|
||||
# disable it entirely. Non-loopback binds sit behind a Cloudflare Tunnel
|
||||
# (idle timeout ~100s) where half-open IS a real failure mode, so keep the
|
||||
# ping at 20/20 to detect it promptly and stay under the tunnel's idle
|
||||
# window.
|
||||
_is_loopback = host in ("127.0.0.1", "localhost", "::1")
|
||||
config = uvicorn.Config(
|
||||
app, host=host, port=port, log_level="warning",
|
||||
|
|
@ -14137,12 +14148,12 @@ def start_server(
|
|||
# decide cookie Secure flags, so we flip proxy_headers on for that
|
||||
# mode.
|
||||
proxy_headers=bool(app.state.auth_required),
|
||||
# Detect half-open WS connections (reverse-proxy 524, dropped
|
||||
# tunnels) within ~20-40s so WebSocketDisconnect fires the
|
||||
# disconnect→reap path. 20s stays under Cloudflare Tunnel's idle
|
||||
# timeout, keeping it warm. Loopback gets a longer window (see above).
|
||||
ws_ping_interval=30.0 if _is_loopback else 20.0,
|
||||
ws_ping_timeout=60.0 if _is_loopback else 20.0,
|
||||
# Half-open detection for public binds only (see above). Loopback
|
||||
# disables the protocol ping (None) so an event-loop stall can never
|
||||
# trigger a false disconnect; a genuinely dead local client is still
|
||||
# reaped via the WebSocketDisconnect → disconnect/reap path.
|
||||
ws_ping_interval=None if _is_loopback else 20.0,
|
||||
ws_ping_timeout=None if _is_loopback else 20.0,
|
||||
)
|
||||
server = uvicorn.Server(config)
|
||||
|
||||
|
|
|
|||
|
|
@ -69,20 +69,48 @@ def _stub_uvicorn(monkeypatch):
|
|||
return captured
|
||||
|
||||
|
||||
def test_start_server_enables_ws_ping_for_half_open_detection(monkeypatch):
|
||||
"""WS ping must be configured so half-open connections (reverse-proxy 524,
|
||||
dropped tunnels) raise WebSocketDisconnect into the reaping path (#32377).
|
||||
def test_start_server_disables_ws_ping_on_loopback(monkeypatch):
|
||||
"""Loopback binds (the Desktop case) MUST disable uvicorn's protocol-level
|
||||
keepalive ping so an event-loop stall can never trigger a false disconnect.
|
||||
|
||||
Loopback binds (the Desktop case) get a longer window to ride out
|
||||
GIL-pressure event-loop stalls (#48445/#50005). The invariant asserted
|
||||
here is that ping stays enabled (non-None, positive) and the timeout is
|
||||
never shorter than the interval — not a frozen literal, which churns every
|
||||
time the window is retuned."""
|
||||
uvicorn's ws ping runs on the same event loop as agent turns. A single
|
||||
synchronous GIL-holding call on a worker thread can starve that loop for
|
||||
minutes, so the loop can't process the pong and uvicorn kills an
|
||||
otherwise-healthy local connection (#53773 "event loop stalled 226.3s",
|
||||
#48445/#50005). On loopback there is no network/proxy path where a
|
||||
half-open connection can occur — a dead local client tears the socket down
|
||||
with a real FIN/RST that surfaces as WebSocketDisconnect regardless — so
|
||||
the ping provides no liveness value and only harms. Assert it is disabled.
|
||||
"""
|
||||
captured = _stub_uvicorn(monkeypatch)
|
||||
|
||||
# Loopback bind => no auth gate, so this reaches the Config constructor.
|
||||
web_server.start_server(host="127.0.0.1", port=0, open_browser=False)
|
||||
|
||||
assert captured["ws_ping_interval"] is None
|
||||
assert captured["ws_ping_timeout"] is None
|
||||
|
||||
|
||||
def test_start_server_enables_ws_ping_for_half_open_detection(monkeypatch):
|
||||
"""Non-loopback (public) binds MUST keep the ws ping enabled so half-open
|
||||
connections (reverse-proxy 524, dropped Cloudflare Tunnel) raise
|
||||
WebSocketDisconnect into the reaping path (#32377).
|
||||
|
||||
The invariant asserted here is that ping stays enabled (non-None, positive)
|
||||
and the timeout is never shorter than the interval — not a frozen literal,
|
||||
which churns every time the window is retuned. Loopback disables the ping
|
||||
(see test_start_server_disables_ws_ping_on_loopback); this covers the
|
||||
public-bind half-open case, so the auth gate is active here.
|
||||
"""
|
||||
captured = _stub_uvicorn(monkeypatch)
|
||||
|
||||
# Non-loopback bind so the _is_loopback branch selects the enabled-ping
|
||||
# window. Neutralize the auth gate so start_server reaches uvicorn.Config
|
||||
# without requiring a registered provider (a real public bind would raise
|
||||
# SystemExit here). The ping window keys off the host, not the auth flag.
|
||||
monkeypatch.setattr(web_server, "should_require_auth", lambda *a, **k: False)
|
||||
web_server.start_server(host="0.0.0.0", port=0, open_browser=False)
|
||||
|
||||
assert captured["ws_ping_interval"] and captured["ws_ping_interval"] > 0
|
||||
assert captured["ws_ping_timeout"] and captured["ws_ping_timeout"] > 0
|
||||
assert captured["ws_ping_timeout"] >= captured["ws_ping_interval"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue