bvandeusenandClaude Opus 5.5
4c150cbc2f
Twenty-eight test files described the code they guard by what it did before a fix. Each is now the property and the failure it prevents, in the present tense: the backfill snapshot, the per-dial calibration query, the rule scope, the lesson trigger filter, the subshell global, the trash owner scope, the version readout fallback, and others. A docstring line that narrated the backup suite's own history is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
126 lines
4.9 KiB
Python
126 lines
4.9 KiB
Python
"""Every request the web UI makes has a deadline.
|
|
|
|
A source-inspection guard in the unit lane — there is no frontend test runner,
|
|
and this is a property of the source rather than of a rendered result, so
|
|
reading the source is the honest way to check it.
|
|
|
|
WHY. `fetch`'s default is to wait as long as the browser will. That is not a
|
|
long timeout, it is the absence of one, and there is no state a surface can
|
|
render for "pending forever" that is not a lie — the spinner that never
|
|
resolves is indistinguishable from work still in progress. The practice names
|
|
`fetch` specifically:
|
|
|
|
When a library's default is "wait indefinitely" — `fetch`, most HTTP
|
|
clients, a bare `await` on a stream — supplying the deadline is part of
|
|
using it, not a hardening pass for later.
|
|
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
FRONTEND = Path(__file__).resolve().parents[1] / "frontend" / "src"
|
|
CLIENT = FRONTEND / "api" / "client.ts"
|
|
|
|
|
|
def _call_text(src: str, start: int) -> str:
|
|
"""The source of one `fetch(...)` call, from its open paren to its close.
|
|
|
|
Naive paren balancing. Adequate because every call site here passes an
|
|
object literal, and a construct complex enough to defeat it is one worth
|
|
looking at by hand anyway.
|
|
"""
|
|
depth = 0
|
|
for i in range(start, len(src)):
|
|
if src[i] == "(":
|
|
depth += 1
|
|
elif src[i] == ")":
|
|
depth -= 1
|
|
if depth == 0:
|
|
return src[start:i + 1]
|
|
return src[start:]
|
|
|
|
|
|
def _fetch_calls() -> list[tuple[Path, str]]:
|
|
calls: list[tuple[Path, str]] = []
|
|
for path in list(FRONTEND.rglob("*.ts")) + list(FRONTEND.rglob("*.vue")):
|
|
src = path.read_text()
|
|
for m in re.finditer(r"\bfetch\(", src):
|
|
calls.append((path, _call_text(src, m.end() - 1)))
|
|
return calls
|
|
|
|
|
|
def test_every_fetch_passes_a_signal():
|
|
"""No bare `fetch` anywhere in the frontend.
|
|
|
|
Stated on the SIGNAL rather than on a timeout value, because the two
|
|
legitimate shapes here produce different values and only share this: an
|
|
ordinary call takes the client's default, a stream bounds its CONNECT and
|
|
then deliberately runs unbounded, and a bulk transfer passes minutes. What
|
|
they must all do is pass something.
|
|
"""
|
|
naked = [
|
|
f"{path.relative_to(FRONTEND)}: {call[:70]}"
|
|
for path, call in _fetch_calls()
|
|
if "signal:" not in call
|
|
]
|
|
assert not naked, (
|
|
"these fetch calls carry no AbortSignal, so they wait forever:\n " + "\n ".join(naked)
|
|
)
|
|
|
|
|
|
def test_the_client_applies_its_deadline_by_default():
|
|
"""The specific regression that would silently undo this.
|
|
|
|
An OPT-IN `timeoutMs` used at one call site leaves every other call
|
|
waiting forever while the mechanism looks present. That shape would not fail the guard above —
|
|
every call would still reach `fetch` through `request()` — so the default
|
|
is pinned here separately.
|
|
|
|
`??` is the operative character: `opts?.timeoutMs || DEFAULT` would treat
|
|
an explicit 0 as "use the default", and `opts?.timeoutMs` alone would
|
|
reinstate the opt-in bug.
|
|
"""
|
|
src = CLIENT.read_text()
|
|
assert "opts?.timeoutMs ?? DEFAULT_TIMEOUT_MS" in src, (
|
|
"request() must fall back to DEFAULT_TIMEOUT_MS — without it the "
|
|
"deadline is opt-in again and almost nothing opts in"
|
|
)
|
|
|
|
|
|
def test_a_timeout_arrives_as_the_error_shape_callers_already_handle():
|
|
"""The deadline's second half: expiry surfaces as a NAMED failure.
|
|
|
|
A raw `DOMException: TimeoutError` reaches `apiErrorMessage(e, fallback)`
|
|
as an object with no `body`, so every catch site in the app would print its
|
|
generic fallback and the timeout would be invisible in exactly the
|
|
situation it exists to expose. Rethrowing as `ApiError` is what makes the
|
|
other ~330 call sites report it without being edited.
|
|
"""
|
|
src = CLIENT.read_text()
|
|
assert 'e.name === "TimeoutError"' in src, (
|
|
"request() must recognise a timeout specifically"
|
|
)
|
|
assert "new ApiError(CLIENT_TIMEOUT_STATUS" in src, (
|
|
"a timeout must be rethrown as ApiError so apiErrorMessage can read it"
|
|
)
|
|
|
|
|
|
def test_a_deliberate_cancellation_is_not_reported_as_a_timeout():
|
|
"""Only `TimeoutError` is converted, never `AbortError`.
|
|
|
|
A caller that cancelled its own request — a superseded search, a closed
|
|
stream — must not have that surfaced to the user as a server failure. The
|
|
guard is that the conversion is gated on the name, which the assertion
|
|
above already pins; this states the intent so the gate is not "simplified"
|
|
into catching every abort.
|
|
"""
|
|
src = CLIENT.read_text()
|
|
convert = src[src.index("async function request<"):]
|
|
convert = convert[:convert.index("\n}")]
|
|
assert "AbortError" not in convert, (
|
|
"request() must not convert AbortError — a deliberate cancellation is "
|
|
"not a timeout"
|
|
)
|