Merge bug/proxy-keepalive-502-retry: stop player POSTs surfacing proxy keepalive 502s

backend/entrypoint.sh

@@ -5,4 +5,9 @@ echo "Applying database migrations..."
 alembic upgrade head
 
 echo "Starting Siftlode API..."
-exec uvicorn app.main:app --host 0.0.0.0 --port 8000 --log-config log_config.json
+# --timeout-keep-alive 75: keep idle upstream connections alive longer than the reverse
+# proxy's idle-reuse window. uvicorn's default (5s) is shorter than Caddy's pooled-keepalive
+# timeout, so Caddy would reuse a connection uvicorn had just closed -> broken pipe / reset
+# on the next request -> 502. Non-idempotent POSTs (the player's progress/state writes,
+# fired every 5s) can't be auto-retried by the proxy, so those 502s reached the user.
+exec uvicorn app.main:app --host 0.0.0.0 --port 8000 --log-config log_config.json --timeout-keep-alive 75

frontend/src/lib/api.ts

@@ -173,43 +173,82 @@ function notifyErrorThrottled(title: string, message: string): void {
   notify({ level: "error", title, message });
 }
 
-async function req(url: string, opts: RequestInit = {}): Promise<any> {
+// Gateway statuses that mean "the upstream connection failed", not "the app rejected the
+// request" — typically a reverse-proxy ↔ uvicorn keepalive connection reset before the
+// request was processed. Safe to retry an idempotent call once on a fresh connection.
+const RETRIABLE_GATEWAY = new Set([502, 503, 504]);
+const delay = (ms: number) => new Promise((res) => window.setTimeout(res, ms));
+
+// `idempotent`: may this request be replayed after a transient gateway/network failure?
+// GETs always can; non-GET callers opt in (e.g. the player's progress/state writes, which
+// are safe to repeat). Used to recover from the keepalive race without surfacing a 502.
+interface ReqConfig {
+  idempotent?: boolean;
+}
+
+async function req(url: string, opts: RequestInit = {}, cfg: ReqConfig = {}): Promise<any> {
   const method = opts.method ?? "GET";
-  let r: Response;
-  try {
-    r = await fetch(url, {
-      credentials: "include",
-      headers: { "Content-Type": "application/json" },
-      ...opts,
-    });
-  } catch (e) {
-    notifyErrorThrottled(
-      "Connection lost",
-      "Couldn't reach the server — it may be restarting. This will clear once it's back."
-    );
-    throw e;
-  }
-  if (!r.ok) {
-    // Capture the server's reason (FastAPI returns `{ detail }`) so callers can react.
-    let detail: string | undefined;
+  const canRetry = cfg.idempotent ?? method === "GET";
+  let attempt = 0;
+
+  for (;;) {
+    let r: Response;
     try {
-      const body = await r.json();
-      if (body && typeof body.detail === "string") detail = body.detail;
-    } catch {
-      /* no JSON body */
+      r = await fetch(url, {
+        credentials: "include",
+        headers: { "Content-Type": "application/json" },
+        ...opts,
+      });
+    } catch (e) {
+      // Network-level failure (incl. a keepalive connection reset surfacing as a TypeError).
+      if (canRetry && attempt === 0) {
+        attempt++;
+        await delay(250);
+        continue;
+      }
+      notifyErrorThrottled(
+        "Connection lost",
+        "Couldn't reach the server — it may be restarting. This will clear once it's back."
+      );
+      throw e;
     }
-    // Surface anything the server definitively refused as a self-explanatory dialog the
-    // user must acknowledge: 5xx (a fault) and 400/409/422 (validation/conflict, with the
-    // server's own reason). 401/403/404 are control flow handled by callers (auth, demo
-    // gating, not-found), so they just throw.
-    if (r.status >= 500) {
-      reportError(detail || `${i18n.t("errors.server")} (${r.status})`);
-    } else if (r.status === 400 || r.status === 409 || r.status === 422) {
-      reportError(detail);
+
+    // A gateway error usually means the upstream connection was reset before our request
+    // was processed — retry idempotent calls once on a fresh connection before surfacing it.
+    if (!r.ok && canRetry && attempt === 0 && RETRIABLE_GATEWAY.has(r.status)) {
+      attempt++;
+      await delay(250);
+      continue;
     }
-    throw new HttpError(r.status, detail);
+
+    if (!r.ok) {
+      // Capture the server's reason (FastAPI returns `{ detail }`) so callers can react.
+      let detail: string | undefined;
+      try {
+        const body = await r.json();
+        if (body && typeof body.detail === "string") detail = body.detail;
+      } catch {
+        /* no JSON body */
+      }
+      // 502/503/504 mean the gateway couldn't reach the app (restarting / connection reset),
+      // not a request the app refused — treat them like a network blip with the soft,
+      // self-clearing toast rather than a blocking modal the user must acknowledge.
+      // A genuine 500 (real fault, carries a JSON detail) still gets the modal, as do
+      // 400/409/422 (validation/conflict). 401/403/404 are caller-handled control flow.
+      if (RETRIABLE_GATEWAY.has(r.status)) {
+        notifyErrorThrottled(
+          "Connection lost",
+          "Couldn't reach the server — it may be restarting. This will clear once it's back."
+        );
+      } else if (r.status >= 500) {
+        reportError(detail || `${i18n.t("errors.server")} (${r.status})`);
+      } else if (r.status === 400 || r.status === 409 || r.status === 422) {
+        reportError(detail);
+      }
+      throw new HttpError(r.status, detail);
+    }
+    return r.status === 204 ? null : r.json();
   }
-  return r.status === 204 ? null : r.json();
 }
 
 // The filter half of the query (everything except paging/sort), shared by the feed and
@@ -392,17 +431,28 @@ export const api = {
     req(`/api/feed/count?${feedQuery(f, 0, 0)}`),
   facets: (f: FeedFilters): Promise<{ counts: Record<string, number> }> =>
     req(`/api/facets?${filterParams(f).toString()}`),
+  // idempotent: setting a state / saving a progress checkpoint is safe to replay, so these
+  // recover from a transient gateway/keepalive 502 instead of surfacing it (see req()).
   setState: (id: string, status: string) =>
-    req(`/api/videos/${id}/state`, { method: "POST", body: JSON.stringify({ status }) }),
-  clearState: (id: string) => req(`/api/videos/${id}/state`, { method: "DELETE" }),
+    req(
+      `/api/videos/${id}/state`,
+      { method: "POST", body: JSON.stringify({ status }) },
+      { idempotent: true }
+    ),
+  clearState: (id: string) =>
+    req(`/api/videos/${id}/state`, { method: "DELETE" }, { idempotent: true }),
   saveProgress: (id: string, positionSeconds: number, durationSeconds: number) =>
-    req(`/api/videos/${id}/progress`, {
-      method: "POST",
-      body: JSON.stringify({
-        position_seconds: Math.floor(positionSeconds),
-        duration_seconds: Math.floor(durationSeconds),
-      }),
-    }),
+    req(
+      `/api/videos/${id}/progress`,
+      {
+        method: "POST",
+        body: JSON.stringify({
+          position_seconds: Math.floor(positionSeconds),
+          duration_seconds: Math.floor(durationSeconds),
+        }),
+      },
+      { idempotent: true }
+    ),
   videoDetail: (id: string): Promise<VideoDetail> => req(`/api/videos/${id}`),
   pauseSync: () => req("/api/sync/pause", { method: "POST" }),
   resumeSync: () => req("/api/sync/resume", { method: "POST" }),