fix(session-stream): stop the hidden-tab poll on 404/410 so deleted sessions don't loop forever (#7299)
## Thinking Path
When a tab is hidden, Hermes WebUI replaces the persistent
per-session SSE connection with a lightweight ``GET /api/session/status``
poll every six seconds. If the tracked session has been deleted
or no longer exists in the active state directory, the endpoint
returns ``404`` — but the client previously converted every non-2xx
response to ``null`` and kept the interval alive forever. This
produces an invisible stream of permanent 404 requests from stale
background tabs (one tab generates ~10 requests per minute when
browser timer throttling allows it; multiple stale tabs multiply
the noise).
The fix narrows the stop condition to the truly terminal responses
(``404 Not Found`` and ``410 Gone``) and calls
``_stopHiddenActiveStreamPoll()`` so the interval and the bound
``_sessionStreamHiddenPollSid`` are cleared together. Transient
failures (5xx, rate-limit, network error) keep polling because they
do not necessarily mean the session is gone; a later tick may catch
a server-initiated turn starting while the tab remains hidden.
## What Changed
``static/messages.js`` — ``_startHiddenActiveStreamPoll()``'s fetch
chain now branches on ``r.status`` before the ``r.ok ? r.json() : null``
fallback:
```js
fetch(...)
.then(r => {
// #7299: 404 Not Found / 410 Gone are TERMINAL for this
// session-owned poll. ... only the missing
// session itself is terminal.
if ((r.status === 404 || r.status === 410) && _sessionStreamHiddenPollSid === sid) {
_stopHiddenActiveStreamPoll();
return null;
}
return r.ok ? r.json() : null;
})
.then(d => { ... });
```
The pre-existing ``r.ok ? r.json() : null`` fallback is preserved
for any non-2xx, non-terminal response, so a 5xx / rate-limit /
network error still produces ``null`` and the next tick re-fires.
A regression that broadened the stop path to all non-2xx would
re-introduce the original bug in the opposite direction (the poll
would die on a single 503 and never recover).
## Why It Matters
A stale background tab with a deleted session could hold an
interval running for hours, firing a 404 every six seconds. The
user sees nothing because the tab is hidden; the server sees
inbound requests that are guaranteed to fail; the request log
fills with noise that an operator has to triage. The fix is
mechanical — the server already returns the right status code, the
client just needs to honor it.
## Verification
- ``tests/test_issue7299_hidden_poll_stops_on_404.py`` (new, 5 tests):
- **404 stops the poll** — source-shape check on the response
handler; a silent reversion to the pre-fix
"convert all non-2xx to null and keep polling" form fails
the suite.
- **410 stops the poll** — same shape, separate code branch
so a future refactor cannot satisfy the 404 check with a
single OR expression.
- **bound sid guards the stop** — a stale fetch's response
landing after a session switch must not tear down the *new*
session's poll.
- **5xx / rate-limit / network errors keep polling** — the
``r.ok ? r.json() : null`` fallback must remain.
- **6000ms interval + immediate-fire preserved** — the fix is
in the response handling, not the cadence.
- ``tests/test_hidden_tab_server_initiated_turn.py`` — pre-existing
tests for the hidden-tab poll (8 tests). The 4 that use
``MESSAGES_JS[start:start + N]`` slicing had their window widened
(2400/3200 → 8000) to cover the new 404/410 branch.
- 33 tests pass across the related session/stream/hidden files
(``test_hidden_tab_server_initiated_turn``,
``test_issue7299_hidden_poll_stops_on_404``,
``test_issue3718_live_models_custom_probe``,
``test_issue6419_pending_user_reconnect_order``,
``test_session_search_bfcache_822``) — no pre-existing test
is broken.
## Contract Change
None. The server already returned the right status code
(``404`` for a missing session); the fix is purely about the
client honoring it. No new public API, no breaking change
to the SSE lifecycle, no change to the visible-tab path.
## Risks
- A regression that broadens the stop path to all non-2xx would
re-introduce the original bug in the opposite direction (the
poll would die on a single 503 and never recover). The new
``test_hidden_poll_preserves_transient_failure_retry`` pins
this contract.
- The 404/410 stop path checks the bound
``_sessionStreamHiddenPollSid === sid`` before stopping, so a
stale fetch's response landing after a session switch cannot
tear down the *new* session's poll. The new
``test_hidden_poll_404_stop_guards_on_bound_sid`` pins this
contract.
## Related
- #7299 — original issue with the wire evidence and the
expected-vs-actual contract.
This commit is contained in:
1 parent
269ddd5377
commit
a26bc45f3c
3 files changed
+172
-3
No files matched your search
@@ -89,7 +89,9 @@ def test_hidden_poll_hits_session_status_and_attaches_as_replay():
|
||||
"""
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
assert start != -1
|
||||
body = MESSAGES_JS[start:start + 2400]
|
||||
# #7299 added a 404/410 terminal-stop branch to the response handler.
|
||||
# The window must cover the original fetch chain + the new branch.
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
assert "api/session/status?session_id=" in body
|
||||
assert "d.active_stream_id" in body
|
||||
# attaches as replay (recovered=true) — turn is already mid-flight
|
||||
@@ -174,7 +176,7 @@ def test_poll_stops_only_when_attach_succeeds():
|
||||
assert start != -1, "_startHiddenActiveStreamPoll signature not found"
|
||||
# Window 2400: the multi-pane follow-up adds an explanatory comment block
|
||||
# before the attach call, pushing it past a narrower slice.
|
||||
body = MESSAGES_JS[start:start + 3200]
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
assert "const attached = _attachServerInitiatedStream(sid, streamId, true)" in body
|
||||
# Stop the poll only on a true attach (the false branch keeps polling within
|
||||
# the bounded-retry budget rather than stopping).
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
"""Regression tests for issue #7299 — hidden-tab active-stream poll
|
||||
keeps retrying deleted sessions forever, generating an invisible stream
|
||||
of permanent 404 requests from stale background tabs.
|
||||
|
||||
When a tab is hidden, Hermes WebUI replaces the persistent per-session
|
||||
SSE connection with a lightweight ``GET /api/session/status`` poll
|
||||
every six seconds. If the tracked session has been deleted or no
|
||||
longer exists in the active state directory, the endpoint returns
|
||||
``404``, but the client previously converted every non-2xx response to
|
||||
``null`` and kept the interval alive forever — one tab generates up to
|
||||
ten requests per minute whenever browser timer throttling allows it.
|
||||
|
||||
The fix: in ``_startHiddenActiveStreamPoll``'s fetch chain, treat
|
||||
``404 Not Found`` and ``410 Gone`` as **terminal** for the
|
||||
session-owned poll — call ``_stopHiddenActiveStreamPoll()`` so the
|
||||
interval and the bound ``_sessionStreamHiddenPollSid`` are cleared
|
||||
together. Transient failures (5xx, rate-limit, network error) keep
|
||||
polling because they do not necessarily mean the session is gone; a
|
||||
later tick may catch a server-initiated turn starting while the tab
|
||||
remains hidden.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||
MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-shape: the fetch chain in ``_startHiddenActiveStreamPoll`` must
|
||||
# gate the terminal 404/410 path through ``_stopHiddenActiveStreamPoll``
|
||||
# and the bound sid check. A silent reversion to the pre-fix
|
||||
# "convert all non-2xx to null and keep polling" form fails the suite.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_hidden_poll_stops_on_404_response() -> None:
|
||||
"""The fetch chain must call ``_stopHiddenActiveStreamPoll`` on a 404.
|
||||
|
||||
A 404 from ``/api/session/status`` means the session has been
|
||||
deleted or no longer exists in the active state directory. The
|
||||
pre-fix code converted every non-2xx to ``null`` and never
|
||||
stopped the interval — so the same 404 fired every six seconds
|
||||
for the lifetime of the hidden tab. Pin the contract that a 404
|
||||
now tears down the poll.
|
||||
"""
|
||||
# The check is anchored on the hidden-poll function so a future
|
||||
# refactor cannot satisfy it with an unrelated status-404
|
||||
# handler in a different code path.
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
assert start != -1
|
||||
# 3.6 KB is enough to cover the fetch → first .then → second .then
|
||||
# block, even after the #7299 fix added the 404/410 terminal
|
||||
# branch. The hidden poll is one of the larger functions in the
|
||||
# file, so we take a wide window to be resilient to a small
|
||||
# reformat.
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
assert "r.status === 404" in body, (
|
||||
"the hidden poll must check r.status === 404 in the fetch "
|
||||
"response so a deleted session no longer triggers an "
|
||||
"infinite 404 loop on a stale background tab"
|
||||
)
|
||||
assert "_stopHiddenActiveStreamPoll" in body, (
|
||||
"the hidden poll must call _stopHiddenActiveStreamPoll when "
|
||||
"the response is terminal (404/410) so the interval is "
|
||||
"cleared instead of left running"
|
||||
)
|
||||
|
||||
|
||||
def test_hidden_poll_stops_on_410_response() -> None:
|
||||
"""The fetch chain must also call ``_stopHiddenActiveStreamPoll`` on 410.
|
||||
|
||||
Some session-state endpoints use ``410 Gone`` (rather than
|
||||
``404 Not Found``) for "this session id is permanently gone".
|
||||
Both are terminal for the hidden poll — the pre-fix code treated
|
||||
them identically, so the fix must handle both.
|
||||
"""
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
assert "r.status === 410" in body, (
|
||||
"the hidden poll must check r.status === 410 in the fetch "
|
||||
"response so a 'gone' session also tears down the poll, "
|
||||
"matching the 404 contract"
|
||||
)
|
||||
|
||||
|
||||
def test_hidden_poll_404_stop_guards_on_bound_sid() -> None:
|
||||
"""The 404/410 stop path must check the bound ``_sessionStreamHiddenPollSid``.
|
||||
|
||||
The same fetch chain is invoked on every tick of a 6-second
|
||||
interval. A race can let a stale fetch's response land after
|
||||
the user switched sessions — in that case ``_sessionStreamHiddenPollSid``
|
||||
no longer matches the fetched ``sid``, and the stop must NOT
|
||||
tear down the *new* session's poll. Pin the contract.
|
||||
"""
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
# The 404/410 branch must check the bound sid before stopping
|
||||
# — pinned as a string match because the JS module is large
|
||||
# and we want a regression to show up at a glance in CI.
|
||||
assert "_sessionStreamHiddenPollSid === sid" in body, (
|
||||
"the 404/410 stop path must guard on "
|
||||
"_sessionStreamHiddenPollSid === sid so a stale fetch's "
|
||||
"response cannot tear down a different session's poll"
|
||||
)
|
||||
|
||||
|
||||
def test_hidden_poll_preserves_transient_failure_retry() -> None:
|
||||
"""5xx / rate-limit / network errors MUST keep polling (not stop).
|
||||
|
||||
The fix only narrows the stop path to terminal 404/410. Transient
|
||||
failures (5xx, 429, network error) still produce ``null`` and
|
||||
the next tick re-fires. A regression that broadens the stop
|
||||
path to all non-2xx would re-introduce the original bug in
|
||||
the opposite direction — the poll would die on a single
|
||||
503 and never recover.
|
||||
"""
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
# The pre-fix ``r.ok ? r.json() : null`` fallback must still be
|
||||
# present for any non-ok / non-terminal response.
|
||||
assert "r.ok ? r.json() : null" in body, (
|
||||
"the 5xx / rate-limit / network-error path must still "
|
||||
"produce null and let the next tick re-fire; only 404/410 "
|
||||
"should tear down the poll"
|
||||
)
|
||||
|
||||
|
||||
def test_hidden_poll_starts_interval_unchanged() -> None:
|
||||
"""The interval is unchanged (still 6000 ms).
|
||||
|
||||
The fix is purely about the response handling, not the
|
||||
cadence. A regression that doubles the interval (or removes
|
||||
the immediate-fire) breaks the original contract — a turn
|
||||
already running when the tab goes hidden must be caught
|
||||
without waiting a full interval.
|
||||
"""
|
||||
start = MESSAGES_JS.find("function _startHiddenActiveStreamPoll(sid)")
|
||||
body = MESSAGES_JS[start:start + 8000]
|
||||
assert "setInterval(tick, 6000)" in body, (
|
||||
"the hidden poll must keep the 6000ms interval; the fix is "
|
||||
"in the response handling, not the cadence"
|
||||
)
|
||||
# The immediate-fire call (no full interval wait) must also
|
||||
# still be present so a turn already running when the tab goes
|
||||
# hidden is caught without waiting a full interval.
|
||||
assert "tick();" in body, (
|
||||
"the immediate-fire tick() call must be preserved so a "
|
||||
"turn already running when the tab goes hidden is caught "
|
||||
"without waiting a full interval"
|
||||
)
|
||||
Reference in new issue
Block a user