frontend/src/api/client.ts

@@ -53,57 +53,120 @@ export function apiErrorMessage(e: unknown, fallback: string): string {
 }
 
 /**
- * A GET, optionally with a deadline.
+ * How long an ordinary JSON call may wait before it is declared failed.
  *
- * `timeoutMs` is OPT-IN rather than defaulted, deliberately. Every existing
- * caller was written against a `fetch` that waits as long as the browser will,
- * and handing them all a deadline in one change would alter behaviour at every
- * call site at once, including ones nobody has looked at. New callers should
- * pass one.
+ * Rule 156: a wait with no deadline is a bug. `fetch`'s own default is to wait
+ * as long as the browser will, which is not a deadline — it is the absence of
+ * one, and it renders as a spinner that never resolves. There is no state a
+ * surface can show for "pending forever" that is not a lie.
  *
- * Why a caller should want it: a wait with no deadline cannot report that it
- * failed. It can only stay pending — which is indistinguishable, to anything
- * rendering it, from "still loading". A surface that has to tell those two
- * apart needs the request to give up on its own.
+ * 30s is chosen to be longer than anything healthy: it has to clear a cold
+ * embedding call and a list view under connection-pool contention (#2384 had
+ * /api/projects fanning 25 concurrent sessions at a 15-connection pool), so
+ * tripping it means something is genuinely wrong rather than merely busy. Slow
+ * BY DESIGN is a different case and passes its own value — see the callers in
+ * SettingsView that do.
  */
-export async function apiGet<T>(path: string, opts?: { timeoutMs?: number }): Promise<T> {
-  const res = await fetch(
-    path,
-    opts?.timeoutMs ? { signal: AbortSignal.timeout(opts.timeoutMs) } : undefined,
+const DEFAULT_TIMEOUT_MS = 30_000;
+
+/** HTTP 408. Not a status any Scribe route returns, so it unambiguously means
+ *  "the client gave up" rather than anything the server said. */
+const CLIENT_TIMEOUT_STATUS = 408;
+
+/**
+ * How long a STREAM may take to answer with its headers.
+ *
+ * Streams are the one case a wall-clock deadline would break: a long-lived SSE
+ * connection is *supposed* to stay open, and `AbortSignal.timeout` would kill
+ * it mid-flight along with the body. But that does not exempt them from rule
+ * 156 — it relocates the deadline. Two different waits are involved:
+ *
+ *   connect — the server answering with headers. CAN fail to answer, so it
+ *             carries this deadline, cleared the moment headers arrive.
+ *   stream  — the body, open indefinitely on purpose. Its failure mode is
+ *             going quiet, which a timeout cannot tell from being idle; that
+ *             is what reconnection and Last-Event-ID are for, not this.
+ *
+ * Reading the connect as exempt because "the stream is long-lived" is the easy
+ * mistake here, and it leaves an unreachable server looking like a quiet one.
+ */
+const STREAM_CONNECT_TIMEOUT_MS = 15_000;
+
+/**
+ * A signal that aborts if headers do not arrive in time, plus the `settle` to
+ * call once they do. After `settle()` the returned signal never fires, so the
+ * stream body runs unbounded — which is the intent.
+ */
+function connectDeadline(base: AbortSignal): { signal: AbortSignal; settle: () => void } {
+  const gate = new AbortController();
+  const timer = setTimeout(
+    () => gate.abort(new DOMException("stream did not connect in time", "TimeoutError")),
+    STREAM_CONNECT_TIMEOUT_MS,
   );
+  return {
+    signal: AbortSignal.any([base, gate.signal]),
+    settle: () => clearTimeout(timer),
+  };
+}
+
+export interface RequestOpts {
+  /** Override the deadline. Pass one when the call is slow BY DESIGN. */
+  timeoutMs?: number;
+}
+
+/**
+ * The one place a request is actually made — every verb below goes through
+ * here, so the deadline cannot be forgotten by adding a sixth.
+ *
+ * EXPIRY SURFACES AS AN `ApiError`, which is rule 156's second half: the
+ * failure has to arrive in the shape the caller already handles. A bare
+ * `DOMException: TimeoutError` would reach `apiErrorMessage(e, fallback)` as
+ * an object with no `body`, so every catch site in the app would report its
+ * generic fallback and the timeout would be invisible in the very situation it
+ * exists to expose. Rethrowing as `ApiError` means ~330 existing call sites
+ * report it correctly without being touched.
+ *
+ * Only a TIMEOUT is converted. A deliberate cancellation aborts with
+ * `AbortError` and is left alone — a caller that cancelled its own request
+ * does not want it reported as a server failure.
+ */
+async function request<T>(path: string, init: RequestInit, opts?: RequestOpts): Promise<T> {
+  const timeoutMs = opts?.timeoutMs ?? DEFAULT_TIMEOUT_MS;
+  let res: Response;
+  try {
+    res = await fetch(path, { ...init, signal: AbortSignal.timeout(timeoutMs) });
+  } catch (e) {
+    if (e instanceof DOMException && e.name === "TimeoutError") {
+      throw new ApiError(CLIENT_TIMEOUT_STATUS, {
+        error: `The server did not answer within ${Math.round(timeoutMs / 1000)}s.`,
+      });
+    }
+    throw e;
+  }
   return handleResponse<T>(res, path);
 }
 
-export async function apiPost<T>(path: string, body: unknown): Promise<T> {
-  const res = await fetch(path, {
-    method: "POST",
-    headers: { "Content-Type": "application/json" },
-    body: JSON.stringify(body),
-  });
-  return handleResponse<T>(res, path);
+/** JSON body headers — the three write verbs sent an identical literal each. */
+const JSON_HEADERS = { "Content-Type": "application/json" };
+
+export function apiGet<T>(path: string, opts?: RequestOpts): Promise<T> {
+  return request<T>(path, {}, opts);
 }
 
-export async function apiPut<T>(path: string, body: unknown): Promise<T> {
-  const res = await fetch(path, {
-    method: "PUT",
-    headers: { "Content-Type": "application/json" },
-    body: JSON.stringify(body),
-  });
-  return handleResponse<T>(res, path);
+export function apiPost<T>(path: string, body: unknown, opts?: RequestOpts): Promise<T> {
+  return request<T>(path, { method: "POST", headers: JSON_HEADERS, body: JSON.stringify(body) }, opts);
 }
 
-export async function apiPatch<T>(path: string, body: unknown): Promise<T> {
-  const res = await fetch(path, {
-    method: "PATCH",
-    headers: { "Content-Type": "application/json" },
-    body: JSON.stringify(body),
-  });
-  return handleResponse<T>(res, path);
+export function apiPut<T>(path: string, body: unknown, opts?: RequestOpts): Promise<T> {
+  return request<T>(path, { method: "PUT", headers: JSON_HEADERS, body: JSON.stringify(body) }, opts);
 }
 
-export async function apiDelete(path: string): Promise<void> {
-  const res = await fetch(path, { method: "DELETE" });
-  return handleResponse<void>(res, path);
+export function apiPatch<T>(path: string, body: unknown, opts?: RequestOpts): Promise<T> {
+  return request<T>(path, { method: "PATCH", headers: JSON_HEADERS, body: JSON.stringify(body) }, opts);
+}
+
+export function apiDelete(path: string, opts?: RequestOpts): Promise<void> {
+  return request<void>(path, { method: "DELETE" }, opts);
 }
 
 // ---------------------------------------------------------------------------
@@ -238,7 +301,14 @@ export function apiSSEStream(
   }
 
   const done = (async () => {
-    const res = await fetch(path, { headers, signal: combinedSignal });
+    // Bounded connect, unbounded stream — see STREAM_CONNECT_TIMEOUT_MS.
+    const connect = connectDeadline(combinedSignal);
+    let res: Response;
+    try {
+      res = await fetch(path, { headers, signal: connect.signal });
+    } finally {
+      connect.settle();
+    }
     if (!res.ok) {
       let body: Record<string, unknown> = {};
       try {
@@ -335,11 +405,19 @@ export async function apiStreamPost(
   body: unknown,
   onChunk: (data: Record<string, unknown>) => void
 ): Promise<void> {
-  const res = await fetch(path, {
-    method: "POST",
-    headers: { "Content-Type": "application/json" },
-    body: JSON.stringify(body),
-  });
+  // Bounded connect, unbounded stream — see STREAM_CONNECT_TIMEOUT_MS.
+  const connect = connectDeadline(new AbortController().signal);
+  let res: Response;
+  try {
+    res = await fetch(path, {
+      method: "POST",
+      headers: JSON_HEADERS,
+      body: JSON.stringify(body),
+      signal: connect.signal,
+    });
+  } finally {
+    connect.settle();
+  }
   if (!res.ok) {
     let errBody: Record<string, unknown> = {};
     try {

frontend/src/api/version.ts

@@ -27,13 +27,15 @@ export interface VersionPayload {
 }
 
 /**
- * The readout exists to answer "what is running?" during an incident, which is
- * exactly when the server may be the thing that is unwell. Without a deadline
- * a failing instance leaves the request pending forever and the surface sits
- * on "still loading" — a blank standing in for `unknown`, which is the failure
- * mode #3127 checklist 12 names by hand. Eight seconds is long enough for a
- * slow-but-alive instance and short enough that a person watching it learns
- * something.
+ * SHORTER than the client's 30s default, deliberately.
+ *
+ * This readout answers "what is running?" during an incident, which is exactly
+ * when the server may be the thing that is unwell — and it is one static field
+ * off a route that does no work, so a healthy instance answers it immediately.
+ * Waiting the full default before saying so would leave a person staring at
+ * "still loading" for half a minute in the moment they are trying to find out
+ * whether the instance is alive at all. Eight seconds clears a slow-but-alive
+ * instance and tells them something quickly when it is not.
  */
 const VERSION_TIMEOUT_MS = 8000;
 

frontend/src/views/SettingsView.vue

@@ -188,6 +188,16 @@ const changingPassword = ref(false);
 const invalidatingSessions = ref(false);
 const exporting = ref(false);
 const restoring = ref(false);
+// Backup, export and restore walk the whole store, so they are slow BY DESIGN
+// and the client's ordinary 30s default would cut them off mid-work. They are
+// still bounded: rule 156 asks for a deadline, not a short one, and "no ceiling
+// at all" is what leaves a restore that died server-side spinning forever.
+const BULK_TRANSFER_TIMEOUT_MS = 10 * 60 * 1000;
+
+function bulkDeadline(): AbortSignal {
+  return AbortSignal.timeout(BULK_TRANSFER_TIMEOUT_MS);
+}
+
 // ── What's running (#3127 checklist 12) ─────────────────────────────────
 // Three states kept apart, because collapsing any two of them is the defect
 // this readout exists to remove: `null` + no error = not asked yet (the Config
@@ -755,7 +765,7 @@ async function exportData(scope: "user" | "full") {
   exporting.value = true;
   try {
     const url = scope === "full" ? "/api/admin/backup" : "/api/admin/backup?scope=user";
-    const res = await fetch(url);
+    const res = await fetch(url, { signal: bulkDeadline() });
     if (!res.ok) {
       const body = await res.json().catch(() => ({ error: `Error ${res.status}` }));
       throw new Error((body as Record<string, string>).error || `Error ${res.status}`);
@@ -780,7 +790,7 @@ const exportingNotes = ref(false);
 async function exportNotes(format: "markdown" | "json") {
   exportingNotes.value = true;
   try {
-    const res = await fetch(`/api/export?format=${format}`);
+    const res = await fetch(`/api/export?format=${format}`, { signal: bulkDeadline() });
     if (!res.ok) throw new Error(`Error ${res.status}`);
     const blob = await res.blob();
     const ext = format === "json" ? "json" : "zip";
@@ -1009,6 +1019,7 @@ async function handleRestoreFile(event: Event) {
       method: "POST",
       headers: { "Content-Type": "application/json" },
       body: JSON.stringify(data),
+      signal: bulkDeadline(),
     });
     if (!res.ok) {
       const body = await res.json().catch(() => ({ error: `Error ${res.status}` }));

tests/test_frontend_request_deadlines.py

@@ -0,0 +1,128 @@
+"""Every request the web UI makes has a deadline (rule 156).
+
+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. Rule 156 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.
+
+Before this guard, no request in the app carried one.
+"""
+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 "
+        "(rule 156):\n  " + "\n  ".join(naked)
+    )
+
+
+def test_the_client_applies_its_deadline_by_default():
+    """The specific regression that would silently undo this.
+
+    An earlier pass (#3329) made `timeoutMs` OPT-IN and used it at exactly one
+    call site, which left ~330 others waiting forever while the mechanism
+    looked present. Reverting to 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():
+    """Rule 156'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"
+    )