diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index c316c70..06ca59c 100644 --- a/frontend/src/lib/api.ts +++ b/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 { +// 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 { 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 }> => 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 => req(`/api/videos/${id}`), pauseSync: () => req("/api/sync/pause", { method: "POST" }), resumeSync: () => req("/api/sync/resume", { method: "POST" }),