diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c07f6f84..4a1fd28a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,15 @@ scan review shows it for such a row. A range that is text itself, like "NEGATIVE", is not turned into bounds, and a marker that already exists keeps its own. (#1313) +- **Several files dropped at once all arrive.** The server takes two + uploads per person at a time and asks the next one to come back in a + second, but the documents page sent three at once and showed that + answer as an error telling you to wait a few minutes. The page now sends + two at a time, and a file the server turns away for a moment, say while + another tab is still uploading, waits and goes again on its own for up + to half a minute before it reports anything. Attaching a new file in the + Coach does the same. Reaching the hourly upload limit now says that + instead of "a few minutes". ### Changed diff --git a/e2e/documents.spec.ts b/e2e/documents.spec.ts index 6516ea389..8393384cc 100644 --- a/e2e/documents.spec.ts +++ b/e2e/documents.spec.ts @@ -820,6 +820,49 @@ test.describe("document vault", () => { await expect(openButton(page, `${marker}-copy.pdf`)).not.toBeVisible(); }); + test("a busy server is waited out and the file still lands", async ({ + page, + isMobile, + }) => { + test.skip(isMobile, "runs once on the desktop project (shared bucket)"); + // The route answers the first POST the way a full upload slot does; the + // queue must retry it after Retry-After instead of showing an error. + let refused = 0; + await page.route("**/api/documents/inbound", async (route) => { + if (route.request().method() === "POST" && refused === 0) { + refused += 1; + await route.fulfill({ + status: 429, + headers: { "Retry-After": "1", "Content-Type": "application/json" }, + body: JSON.stringify({ + data: null, + error: "Too many uploads are already in progress.", + meta: { + errorCode: "documents.inbound.uploadBusy", + reason: "uploadBusy", + }, + }), + }); + return; + } + await route.fallback(); + }); + await page.goto("/documents"); + const marker = `busy-probe-${Date.now()}`; + + await uploadViaPicker(page, `${marker}.pdf`, uniquePdf(marker)); + await page + .getByRole("searchbox", { name: "Search documents" }) + .fill(marker); + await expect(openButton(page, `${marker}.pdf`)).toBeVisible({ + timeout: 15_000, + }); + expect(refused).toBe(1); + await expect( + page.locator('[data-slot="document-upload-card"]'), + ).toHaveCount(0); + }); + // ── Admin cap: commit-on-Enter + rejection copy naming the limit ────── test("a 1 MB admin cap rejects an oversized upload with the limit in the copy", async ({ diff --git a/messages/de.json b/messages/de.json index 4ab05dd39..3d1972dc1 100644 --- a/messages/de.json +++ b/messages/de.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Dieser Dateityp wird nicht unterstützt.", "purged": "Dieses Dokument wurde endgültig entfernt und kann nicht wiederhergestellt werden.", "duplicateExists": "Ein identisches Dokument ist bereits gespeichert.", - "rateLimited": "Zu viele Uploads. Versuche es in ein paar Minuten erneut.", + "uploadBusy": "Der Server war mit anderen Uploads beschäftigt. Versuche es jetzt erneut.", + "rateLimited": "Das Upload-Limit für diese Stunde ist erreicht. Versuche es später erneut.", "generic": "Upload fehlgeschlagen. Bitte erneut versuchen." }, "toast": { diff --git a/messages/en.json b/messages/en.json index d2120bfd1..2e22e2384 100644 --- a/messages/en.json +++ b/messages/en.json @@ -12011,7 +12011,8 @@ "unsupportedType": "This file type is not supported.", "purged": "This document was permanently removed and cannot be restored.", "duplicateExists": "An identical document is already stored.", - "rateLimited": "Too many uploads right now. Try again in a few minutes.", + "uploadBusy": "The server was busy with other uploads. Try again now.", + "rateLimited": "You have reached the upload limit for this hour. Try again later.", "generic": "Upload failed. Please try again." }, "toast": { diff --git a/messages/es.json b/messages/es.json index 4c7b923dd..784875196 100644 --- a/messages/es.json +++ b/messages/es.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Este tipo de archivo no es compatible.", "purged": "Este documento se eliminó definitivamente y no se puede restaurar.", "duplicateExists": "Ya hay guardado un documento idéntico.", - "rateLimited": "Demasiadas subidas. Inténtalo de nuevo en unos minutos.", + "uploadBusy": "El servidor estaba ocupado con otras subidas. Inténtalo de nuevo ahora.", + "rateLimited": "Has alcanzado el límite de subidas de esta hora. Inténtalo de nuevo más tarde.", "generic": "La subida falló. Inténtalo de nuevo." }, "toast": { diff --git a/messages/fr.json b/messages/fr.json index 4d73292b7..594bd8707 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Ce type de fichier n'est pas pris en charge.", "purged": "Ce document a été définitivement supprimé et ne peut pas être restauré.", "duplicateExists": "Un document identique est déjà enregistré.", - "rateLimited": "Trop de téléversements. Réessayez dans quelques minutes.", + "uploadBusy": "Le serveur était occupé par d’autres téléversements. Réessayez maintenant.", + "rateLimited": "Vous avez atteint la limite de téléversements pour cette heure. Réessayez plus tard.", "generic": "Le téléversement a échoué. Veuillez réessayer." }, "toast": { diff --git a/messages/it.json b/messages/it.json index f479729da..a2d6decf1 100644 --- a/messages/it.json +++ b/messages/it.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Questo tipo di file non è supportato.", "purged": "Questo documento è stato rimosso definitivamente e non può essere ripristinato.", "duplicateExists": "Un documento identico è già archiviato.", - "rateLimited": "Troppi caricamenti. Riprova tra qualche minuto.", + "uploadBusy": "Il server era occupato con altri caricamenti. Riprova adesso.", + "rateLimited": "Hai raggiunto il limite di caricamenti per quest’ora. Riprova più tardi.", "generic": "Caricamento non riuscito. Riprova." }, "toast": { diff --git a/messages/ko.json b/messages/ko.json index 91c7e58d2..98e810f3e 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -12011,7 +12011,8 @@ "unsupportedType": "지원하지 않는 파일 형식이에요.", "purged": "이 문서는 완전히 삭제되어 복구할 수 없어요.", "duplicateExists": "같은 문서가 이미 저장돼 있어요.", - "rateLimited": "지금은 업로드가 너무 많아요. 몇 분 뒤에 다시 시도해 주세요.", + "uploadBusy": "서버가 다른 업로드를 처리하느라 바빴어요. 지금 다시 시도해 주세요.", + "rateLimited": "이번 시간의 업로드 한도에 도달했어요. 나중에 다시 시도해 주세요.", "generic": "업로드하지 못했어요. 다시 시도해 주세요." }, "toast": { diff --git a/messages/nl.json b/messages/nl.json index d0a7df7f3..6150f3a99 100644 --- a/messages/nl.json +++ b/messages/nl.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Dit bestandstype wordt niet ondersteund.", "purged": "Dit document is definitief verwijderd en kan niet worden hersteld.", "duplicateExists": "Er is al een identiek document opgeslagen.", - "rateLimited": "Te veel uploads op dit moment. Probeer het over een paar minuten opnieuw.", + "uploadBusy": "De server was bezig met andere uploads. Probeer het nu opnieuw.", + "rateLimited": "Je hebt de uploadlimiet voor dit uur bereikt. Probeer het later opnieuw.", "generic": "Uploaden is mislukt. Probeer het opnieuw." }, "toast": { diff --git a/messages/pl.json b/messages/pl.json index fe6c9bc6a..b5b4b8c3a 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Ten typ pliku nie jest obsługiwany.", "purged": "Ten dokument został trwale usunięty i nie można go przywrócić.", "duplicateExists": "Identyczny dokument jest już zapisany.", - "rateLimited": "Zbyt wiele przesyłań. Spróbuj ponownie za kilka minut.", + "uploadBusy": "Serwer był zajęty innymi przesyłaniami. Spróbuj ponownie teraz.", + "rateLimited": "Osiągnięto limit przesyłań na tę godzinę. Spróbuj ponownie później.", "generic": "Przesyłanie nie powiodło się. Spróbuj ponownie." }, "toast": { diff --git a/messages/pt-BR.json b/messages/pt-BR.json index 1a7643b7f..f748f31d0 100644 --- a/messages/pt-BR.json +++ b/messages/pt-BR.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Este tipo de arquivo não é suportado.", "purged": "Este documento foi removido permanentemente e não pode ser restaurado.", "duplicateExists": "Um documento idêntico já está armazenado.", - "rateLimited": "Envios demais agora. Tente novamente em alguns minutos.", + "uploadBusy": "O servidor estava ocupado com outros envios. Tente novamente agora.", + "rateLimited": "Você atingiu o limite de envios desta hora. Tente novamente mais tarde.", "generic": "Falha no envio. Tente novamente." }, "toast": { diff --git a/messages/ru.json b/messages/ru.json index 4c4271bf6..8aa60b5ae 100644 --- a/messages/ru.json +++ b/messages/ru.json @@ -12011,7 +12011,8 @@ "unsupportedType": "Этот тип файла не поддерживается.", "purged": "Этот документ удалён навсегда и не может быть восстановлен.", "duplicateExists": "Идентичный документ уже сохранён.", - "rateLimited": "Слишком много загрузок сейчас. Попробуйте через несколько минут.", + "uploadBusy": "Сервер был занят другими загрузками. Попробуйте ещё раз сейчас.", + "rateLimited": "Достигнут лимит загрузок на этот час. Попробуйте позже.", "generic": "Загрузка не удалась. Попробуйте ещё раз." }, "toast": { diff --git a/src/app/api/documents/inbound/__tests__/route.test.ts b/src/app/api/documents/inbound/__tests__/route.test.ts index a68945788..8dbb8881c 100644 --- a/src/app/api/documents/inbound/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/__tests__/route.test.ts @@ -159,8 +159,8 @@ import { getAiCapability } from "@/lib/ai/capabilities/gate"; import { acquireDocumentUploadSlot, DOCUMENT_UPLOAD_GLOBAL_CONCURRENCY, - DOCUMENT_UPLOAD_PER_USER_CONCURRENCY, } from "@/lib/documents/upload-policy"; +import { DOCUMENT_UPLOAD_PER_USER_CONCURRENCY } from "@/lib/documents/inbound-constants"; const post = POST as unknown as (r: Request) => Promise; const get = GET as unknown as (r: Request) => Promise; @@ -413,6 +413,8 @@ describe("POST /api/documents/inbound (vault upload)", () => { try { const res = await post(request); expect(res.status).toBe(429); + // The web upload queue waits this long and retries the same file. + expect(res.headers.get("Retry-After")).toBe("1"); await expect(res.json()).resolves.toMatchObject({ meta: { reason: "uploadBusy" }, }); @@ -431,6 +433,8 @@ describe("POST /api/documents/inbound (vault upload)", () => { try { const res = await post(mkUpload()); expect(res.status).toBe(429); + // The web upload queue waits this long and retries the same file. + expect(res.headers.get("Retry-After")).toBe("1"); await expect(res.json()).resolves.toMatchObject({ meta: { reason: "uploadBusy" }, }); diff --git a/src/components/documents/__tests__/upload-busy-retry.test.ts b/src/components/documents/__tests__/upload-busy-retry.test.ts new file mode 100644 index 000000000..2a7e1af34 --- /dev/null +++ b/src/components/documents/__tests__/upload-busy-retry.test.ts @@ -0,0 +1,157 @@ +/** + * The upload queue's answer to the route's busy signal (429 `uploadBusy`, + * `Retry-After: 1`). Before this, the vault turned that one-second refusal + * into an error card that told the person to come back in a few minutes. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + busyRetryDelayMs, + parseRetryAfterSeconds, + UPLOAD_BUSY_BUDGET_MS, + UPLOAD_BUSY_MAX_ATTEMPTS, + uploadWithBusyRetry, +} from "../upload-busy-retry"; +import type { UploadResult } from "../vault-utils"; + +const busy = (retryAfterSeconds?: number): UploadResult => ({ + ok: false, + reason: "uploadBusy", + retryAfterSeconds, +}); +const stored = { + ok: true, + duplicate: false, + document: { id: "doc-1" }, +} as unknown as UploadResult; + +const noJitter = () => 0; + +describe("parseRetryAfterSeconds", () => { + it("reads whole seconds", () => { + expect(parseRetryAfterSeconds("1")).toBe(1); + expect(parseRetryAfterSeconds(" 12 ")).toBe(12); + }); + + it("ignores an absent, dated or malformed header", () => { + expect(parseRetryAfterSeconds(null)).toBeUndefined(); + expect(parseRetryAfterSeconds("")).toBeUndefined(); + expect( + parseRetryAfterSeconds("Wed, 21 Oct 2026 07:28:00 GMT"), + ).toBeUndefined(); + expect(parseRetryAfterSeconds("-3")).toBeUndefined(); + expect(parseRetryAfterSeconds("1.5")).toBeUndefined(); + }); +}); + +describe("busyRetryDelayMs", () => { + it("waits Retry-After, clamped to 1 to 10 seconds", () => { + expect(busyRetryDelayMs(1, noJitter)).toBe(1000); + expect(busyRetryDelayMs(0, noJitter)).toBe(1000); + expect(busyRetryDelayMs(undefined, noJitter)).toBe(1000); + expect(busyRetryDelayMs(4, noJitter)).toBe(4000); + expect(busyRetryDelayMs(3600, noJitter)).toBe(10_000); + }); + + it("adds under half a second of jitter", () => { + expect(busyRetryDelayMs(1, () => 0.999)).toBe(1499); + }); +}); + +describe("uploadWithBusyRetry", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + }); + + it("retries a busy answer after Retry-After and returns the success", async () => { + const attempt = vi + .fn<() => Promise>() + .mockResolvedValueOnce(busy(1)) + .mockResolvedValueOnce(stored); + const onRetry = vi.fn(); + + const pending = uploadWithBusyRetry(attempt, { random: noJitter, onRetry }); + await vi.advanceTimersByTimeAsync(999); + expect(attempt).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + + await expect(pending).resolves.toBe(stored); + expect(attempt).toHaveBeenCalledTimes(2); + expect(onRetry).toHaveBeenCalledTimes(1); + }); + + it("gives up after the attempt cap and hands back the busy result", async () => { + const attempt = vi + .fn<() => Promise>() + .mockResolvedValue(busy(1)); + + const pending = uploadWithBusyRetry(attempt, { random: noJitter }); + await vi.advanceTimersByTimeAsync(UPLOAD_BUSY_BUDGET_MS); + + await expect(pending).resolves.toMatchObject({ reason: "uploadBusy" }); + expect(attempt).toHaveBeenCalledTimes(UPLOAD_BUSY_MAX_ATTEMPTS); + }); + + it("stops before a wait that would overrun the time budget", async () => { + // 10 s per wait (clamped): attempts at 0, 10 and 20 s; a third wait + // would end at 30 s plus jitter, past the budget. + const attempt = vi + .fn<() => Promise>() + .mockResolvedValue(busy(60)); + + const pending = uploadWithBusyRetry(attempt, { random: () => 0.5 }); + await vi.advanceTimersByTimeAsync(UPLOAD_BUSY_BUDGET_MS * 2); + + await expect(pending).resolves.toMatchObject({ reason: "uploadBusy" }); + expect(attempt).toHaveBeenCalledTimes(3); + }); + + it("never retries any other failure, the hourly limit included", async () => { + for (const failure of [ + { ok: false, reason: "rateLimited" }, + { ok: false, reason: "generic" }, + { ok: false, reason: "fileTooLarge" }, + ] as UploadResult[]) { + const attempt = vi + .fn<() => Promise>() + .mockResolvedValue(failure); + await expect(uploadWithBusyRetry(attempt)).resolves.toBe(failure); + expect(attempt).toHaveBeenCalledTimes(1); + } + }); + + it("an abort during the wait clears the timer and sends nothing more", async () => { + const attempt = vi + .fn<() => Promise>() + .mockResolvedValue(busy(5)); + const controller = new AbortController(); + + const pending = uploadWithBusyRetry(attempt, { + signal: controller.signal, + random: noJitter, + }); + await vi.advanceTimersByTimeAsync(1000); + controller.abort(); + + await expect(pending).resolves.toMatchObject({ reason: "uploadBusy" }); + expect(vi.getTimerCount()).toBe(0); + await vi.advanceTimersByTimeAsync(UPLOAD_BUSY_BUDGET_MS); + expect(attempt).toHaveBeenCalledTimes(1); + }); + + it("an already-aborted signal makes no second attempt", async () => { + const attempt = vi + .fn<() => Promise>() + .mockResolvedValue(busy(1)); + const controller = new AbortController(); + controller.abort(); + + await expect( + uploadWithBusyRetry(attempt, { signal: controller.signal }), + ).resolves.toMatchObject({ reason: "uploadBusy" }); + expect(attempt).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/components/documents/__tests__/use-document-upload-busy.test.ts b/src/components/documents/__tests__/use-document-upload-busy.test.ts new file mode 100644 index 000000000..bbc3dc097 --- /dev/null +++ b/src/components/documents/__tests__/use-document-upload-busy.test.ts @@ -0,0 +1,58 @@ +/** + * The vault's XHR transport carries the busy refusal's `Retry-After` into + * the upload result, which is what the queue's retry waits on. Without the + * header the retry still happens, but at the floor delay instead of the one + * the server named. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { uploadViaXhr } from "../use-document-upload"; + +class BusyXhr { + status = 0; + responseText = ""; + upload = { addEventListener: () => {} }; + private listeners = new Map void>(); + + open(): void {} + setRequestHeader(): void {} + + getResponseHeader(name: string): string | null { + return name.toLowerCase() === "retry-after" ? "3" : null; + } + + addEventListener(type: string, listener: () => void): void { + this.listeners.set(type, listener); + } + + send(): void { + this.status = 429; + this.responseText = JSON.stringify({ + data: null, + error: "Too many uploads are already in progress. Try again shortly.", + meta: { errorCode: "documents.inbound.uploadBusy", reason: "uploadBusy" }, + }); + this.listeners.get("load")?.(); + } +} + +beforeEach(() => { + vi.stubGlobal("XMLHttpRequest", BusyXhr); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe("uploadViaXhr on a busy server", () => { + it("reports uploadBusy with the server's Retry-After", async () => { + const file = new File([new Uint8Array([1])], "scan.pdf", { + type: "application/pdf", + }); + await expect(uploadViaXhr(file, {}, () => {})).resolves.toEqual({ + ok: false, + reason: "uploadBusy", + retryAfterSeconds: 3, + }); + }); +}); diff --git a/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts b/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts index bb7925998..1b795dac4 100644 --- a/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts +++ b/src/components/documents/__tests__/use-document-upload-insecure-context.test.ts @@ -69,6 +69,10 @@ class FakeXhr { this.request.headers[name] = value; } + getResponseHeader(): string | null { + return null; + } + addEventListener(type: string, listener: () => void): void { this.listeners.set(type, listener); } diff --git a/src/components/documents/__tests__/vault-utils.test.ts b/src/components/documents/__tests__/vault-utils.test.ts index d881f7a5e..2fc1cbf5d 100644 --- a/src/components/documents/__tests__/vault-utils.test.ts +++ b/src/components/documents/__tests__/vault-utils.test.ts @@ -304,6 +304,27 @@ describe("upload response contract (§3.2)", () => { }); expect(parseUploadResponse(500, "")).toMatchObject({ reason: "generic" }); }); + + it("tells the busy refusal apart from the hourly limit and keeps its Retry-After", () => { + const busyBody = JSON.stringify({ + data: null, + error: "Too many uploads are already in progress.", + meta: { errorCode: "documents.inbound.uploadBusy", reason: "uploadBusy" }, + }); + expect(parseUploadResponse(429, busyBody, "1")).toEqual({ + ok: false, + reason: "uploadBusy", + retryAfterSeconds: 1, + }); + const limitBody = JSON.stringify({ + data: null, + error: "Too many uploads. Try again later.", + meta: { errorCode: "documents.inbound.rateLimited" }, + }); + expect(parseUploadResponse(429, limitBody, "1800")).toMatchObject({ + reason: "rateLimited", + }); + }); }); describe("shift-click range selection", () => { diff --git a/src/components/documents/document-card.tsx b/src/components/documents/document-card.tsx index 1fa4b2f2b..45465fd9b 100644 --- a/src/components/documents/document-card.tsx +++ b/src/components/documents/document-card.tsx @@ -469,6 +469,9 @@ export function UploadStateCard({ case "duplicateExists": failureCopy = t("documents.error.duplicateExists"); break; + case "uploadBusy": + failureCopy = t("documents.error.uploadBusy"); + break; case "rateLimited": failureCopy = t("documents.error.rateLimited"); break; diff --git a/src/components/documents/upload-busy-retry.ts b/src/components/documents/upload-busy-retry.ts new file mode 100644 index 000000000..a7cff7963 --- /dev/null +++ b/src/components/documents/upload-busy-retry.ts @@ -0,0 +1,94 @@ +/** + * Waiting out the upload route's busy signal. + * + * The inbound route admits a bounded number of uploads per person and per + * process (`acquireDocumentUploadSlot` in `src/lib/documents/upload-policy.ts`) + * and answers anything beyond that with 429, `meta.reason: "uploadBusy"` and + * `Retry-After: 1`. That is "a slot frees up in a second", not a quota, so the + * client retries the same file on its own instead of showing an error: a + * person dropping several files, or uploading from two tabs, should never see + * one. Only after the budget below runs out does the row turn into an error. + * + * The hourly upload limit (`documents.inbound.rateLimited`) is a different + * 429 and is never retried here. + */ +import type { UploadResult } from "./vault-utils"; + +/** Attempts per file, the first one included. */ +export const UPLOAD_BUSY_MAX_ATTEMPTS = 6; + +/** Longest a file keeps retrying before it gives up, measured from the first attempt. */ +export const UPLOAD_BUSY_BUDGET_MS = 30_000; + +const MIN_WAIT_SECONDS = 1; +const MAX_WAIT_SECONDS = 10; +/** Spread two waiting files apart so they do not collide again on the same tick. */ +const MAX_JITTER_MS = 500; + +/** `Retry-After` as whole seconds; anything else (HTTP-date, junk, absent) is undefined. */ +export function parseRetryAfterSeconds( + header: string | null | undefined, +): number | undefined { + if (!header || !/^\s*\d+\s*$/u.test(header)) return undefined; + return Number.parseInt(header, 10); +} + +/** How long to wait before the next attempt: `Retry-After` clamped to 1–10 s, plus jitter. */ +export function busyRetryDelayMs( + retryAfterSeconds: number | undefined, + random: () => number = Math.random, +): number { + const seconds = Math.min( + MAX_WAIT_SECONDS, + Math.max(MIN_WAIT_SECONDS, retryAfterSeconds ?? MIN_WAIT_SECONDS), + ); + return seconds * 1000 + Math.floor(random() * MAX_JITTER_MS); +} + +/** Resolves true after `ms`, or false as soon as `signal` aborts (the timer is cleared). */ +function waitUnlessAborted(ms: number, signal?: AbortSignal): Promise { + if (signal?.aborted) return Promise.resolve(false); + return new Promise((resolve) => { + const onAbort = () => { + clearTimeout(timer); + resolve(false); + }; + const timer = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(true); + }, ms); + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} + +export interface BusyRetryOptions { + /** Aborting stops any further attempt; the last result is returned as is. */ + signal?: AbortSignal; + /** Called before every attempt after the first. */ + onRetry?: () => void; + random?: () => number; + now?: () => number; +} + +/** + * Run `attempt` and repeat it while the server answers `uploadBusy`, within + * {@link UPLOAD_BUSY_MAX_ATTEMPTS} and {@link UPLOAD_BUSY_BUDGET_MS}. Every + * other result, success or failure, comes back after the attempt that + * produced it. + */ +export async function uploadWithBusyRetry( + attempt: () => Promise, + options: BusyRetryOptions = {}, +): Promise { + const now = options.now ?? Date.now; + const started = now(); + for (let made = 1; ; made += 1) { + const result = await attempt(); + if (result.ok || result.reason !== "uploadBusy") return result; + if (made >= UPLOAD_BUSY_MAX_ATTEMPTS) return result; + const delay = busyRetryDelayMs(result.retryAfterSeconds, options.random); + if (now() - started + delay > UPLOAD_BUSY_BUDGET_MS) return result; + if (!(await waitUnlessAborted(delay, options.signal))) return result; + options.onRetry?.(); + } +} diff --git a/src/components/documents/use-document-upload.ts b/src/components/documents/use-document-upload.ts index bd7a9e1c7..7eb16d7b2 100644 --- a/src/components/documents/use-document-upload.ts +++ b/src/components/documents/use-document-upload.ts @@ -1,7 +1,8 @@ "use client"; /** - * The vault's upload manager: a client-side queue with concurrency 3, one + * The vault's upload manager: a client-side queue as wide as the server's + * per-person upload admission (`DOCUMENT_UPLOAD_PER_USER_CONCURRENCY`), one * multipart XHR POST per file (XHR because it exposes real upload-progress * events with zero dependencies — fetch upload streaming is not portable), * client-side image downscaling, and per-file terminal states. @@ -17,22 +18,33 @@ * toasts "already stored" and hands the row id to the timeline for a brief * highlight. This is the one call site that must read success-side `meta`; * `parseUploadResponse` in `vault-utils.ts` owns that parse. + * + * Busy contract: when the server is still holding other uploads (another + * tab, another device, another person on the same process) it answers 429 + * `uploadBusy` with `Retry-After`. The file stays in its uploading row and + * is sent again (`upload-busy-retry.ts`); only a spent retry budget turns it + * into an error. Leaving the page cancels any pending retry. */ import { useCallback, useEffect, useRef, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import { useTranslations } from "@/lib/i18n/context"; +import { DOCUMENT_UPLOAD_PER_USER_CONCURRENCY } from "@/lib/documents/inbound-constants"; import { randomId } from "@/lib/random-id"; import { invalidateKeys, queryKeys } from "@/lib/query-keys"; +import { uploadWithBusyRetry } from "./upload-busy-retry"; import { parseUploadResponse, type UploadFailure, type UploadResult, } from "./vault-utils"; -/** Client-side parallelism — matches the plan's NAS memory envelope. */ -const CONCURRENCY = 3; +/** + * Client-side parallelism. Any wider and the queue's own surplus files are + * refused as busy by the server, which admits this many per person. + */ +const CONCURRENCY = DOCUMENT_UPLOAD_PER_USER_CONCURRENCY; /** Longest-edge ceiling for the client-side image downscale. */ const IMAGE_MAX_EDGE = 3000; @@ -133,7 +145,13 @@ export function uploadViaXhr( } }); xhr.addEventListener("load", () => { - resolve(parseUploadResponse(xhr.status, xhr.responseText)); + resolve( + parseUploadResponse( + xhr.status, + xhr.responseText, + xhr.getResponseHeader("Retry-After"), + ), + ); }); xhr.addEventListener("error", () => resolve({ ok: false, reason: "generic" }), @@ -197,11 +215,16 @@ export function useDocumentUpload( >([]); const mountedRef = useRef(true); const highlightTimer = useRef | null>(null); + /** Aborted on unmount so no busy-retry timer outlives the page. */ + const retryAbortRef = useRef(null); useEffect(() => { mountedRef.current = true; + const retryAbort = new AbortController(); + retryAbortRef.current = retryAbort; return () => { mountedRef.current = false; + retryAbort.abort(); if (highlightTimer.current) clearTimeout(highlightTimer.current); }; }, []); @@ -259,8 +282,15 @@ export function useDocumentUpload( void (async () => { try { const prepared = await downscaleImage(next.file); - const result = await uploadViaXhr(prepared, next.options, (f) => - patchItem(next.localId, { progress: f }), + const result = await uploadWithBusyRetry( + () => + uploadViaXhr(prepared, next.options, (f) => + patchItem(next.localId, { progress: f }), + ), + { + signal: retryAbortRef.current?.signal, + onRetry: () => patchItem(next.localId, { progress: 0 }), + }, ); settle(next.localId, result); } finally { diff --git a/src/components/documents/vault-utils.ts b/src/components/documents/vault-utils.ts index b87e3cfaf..52e6af4a5 100644 --- a/src/components/documents/vault-utils.ts +++ b/src/components/documents/vault-utils.ts @@ -19,6 +19,8 @@ import { import type { DocumentVaultFilters } from "@/lib/query-keys/documents"; import { userDayKey } from "@/lib/tz/format"; +import { parseRetryAfterSeconds } from "./upload-busy-retry"; + const KIND_SET = new Set(INBOUND_DOCUMENT_KINDS); /** @@ -378,8 +380,10 @@ export function resolveBulkShareDocuments( /** * The §3.2 failure reasons the upload/restore paths translate client-side, - * plus the transport-level fallbacks. `rateLimited` maps 429; `generic` - * covers everything unclassifiable (proxy-stripped bodies included). + * plus the transport-level fallbacks. `uploadBusy` is the route's + * concurrency refusal (429 with `Retry-After`, retried by the client before + * it ever shows); `rateLimited` is every other 429; `generic` covers + * everything unclassifiable (proxy-stripped bodies included). */ export type UploadFailureReason = | "fileTooLarge" @@ -387,6 +391,7 @@ export type UploadFailureReason = | "unsupportedType" | "purged" | "duplicateExists" + | "uploadBusy" | "rateLimited" | "generic"; @@ -400,6 +405,8 @@ export interface UploadFailure { usedBytes?: number; /** §3.2 — present on `duplicateExists` (restore-conflict shape). */ existingId?: string; + /** Present on `uploadBusy`: the response's `Retry-After`, in seconds. */ + retryAfterSeconds?: number; } export interface UploadSuccess { @@ -431,8 +438,12 @@ function numberOrUndefined(v: unknown): number | undefined { export function classifyUploadFailure( status: number, meta: Record | undefined, + retryAfterSeconds?: number, ): UploadFailure { const reason = typeof meta?.reason === "string" ? meta.reason : ""; + if (status === 429 && reason === "uploadBusy") { + return { ok: false, reason: "uploadBusy", retryAfterSeconds }; + } if (REASONS.has(reason)) { return { ok: false, @@ -461,6 +472,7 @@ export function classifyUploadFailure( export function parseUploadResponse( status: number, bodyText: string, + retryAfterHeader?: string | null, ): UploadResult { let envelope: { data?: unknown; @@ -486,7 +498,11 @@ export function parseUploadResponse( } return { ok: false, reason: "generic" }; } - return classifyUploadFailure(status, envelope?.meta); + return classifyUploadFailure( + status, + envelope?.meta, + parseRetryAfterSeconds(retryAfterHeader), + ); } /** What a keypress on a focused document card does. */ diff --git a/src/components/insights/coach-panel/__tests__/upload-attachment.test.ts b/src/components/insights/coach-panel/__tests__/upload-attachment.test.ts new file mode 100644 index 000000000..aa9864364 --- /dev/null +++ b/src/components/insights/coach-panel/__tests__/upload-attachment.test.ts @@ -0,0 +1,76 @@ +/** + * A file attached in the Coach while the vault is still uploading elsewhere + * gets the route's busy answer first; it must wait and land, not toast. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const apiFetchRaw = vi.fn(); +vi.mock("@/lib/api/api-fetch", () => ({ + apiFetchRaw: (...args: unknown[]) => apiFetchRaw(...args), +})); + +import { uploadCoachAttachment } from "../upload-attachment"; + +function answer(status: number, body: unknown, retryAfter?: string): Response { + return new Response(JSON.stringify(body), { + status, + headers: retryAfter ? { "Retry-After": retryAfter } : {}, + }); +} + +const busyBody = { + data: null, + error: "Too many uploads are already in progress. Try again shortly.", + meta: { errorCode: "documents.inbound.uploadBusy", reason: "uploadBusy" }, +}; + +beforeEach(() => { + vi.useFakeTimers(); + apiFetchRaw.mockReset(); +}); +afterEach(() => { + vi.useRealTimers(); +}); + +describe("uploadCoachAttachment", () => { + it("waits out a busy answer and returns the stored document", async () => { + apiFetchRaw + .mockResolvedValueOnce(answer(429, busyBody, "1")) + .mockResolvedValueOnce( + answer(201, { data: { id: "doc-1" }, error: null }), + ); + const file = new File(["%PDF-1.4"], "letter.pdf", { + type: "application/pdf", + }); + + const pending = uploadCoachAttachment(file, undefined); + await vi.advanceTimersByTimeAsync(1500); + const upload = await pending; + + expect(upload.result).toMatchObject({ + ok: true, + document: { id: "doc-1" }, + }); + expect(apiFetchRaw).toHaveBeenCalledTimes(2); + // Each attempt is its own request with its own key and a fresh body. + const [first, second] = apiFetchRaw.mock.calls.map( + (call) => (call[1] as RequestInit).headers as Record, + ); + expect(first!["Idempotency-Key"]).not.toBe(second!["Idempotency-Key"]); + }); + + it("an unmount during the wait sends nothing more", async () => { + apiFetchRaw.mockResolvedValue(answer(429, busyBody, "1")); + const controller = new AbortController(); + const file = new File(["%PDF-1.4"], "letter.pdf"); + + const pending = uploadCoachAttachment(file, controller.signal); + await vi.advanceTimersByTimeAsync(10); + controller.abort(); + const upload = await pending; + + expect(upload.result).toMatchObject({ ok: false, reason: "uploadBusy" }); + await vi.advanceTimersByTimeAsync(30_000); + expect(apiFetchRaw).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/components/insights/coach-panel/coach-conversation.tsx b/src/components/insights/coach-panel/coach-conversation.tsx index 79fa0bd9e..90a7a61cd 100644 --- a/src/components/insights/coach-panel/coach-conversation.tsx +++ b/src/components/insights/coach-panel/coach-conversation.tsx @@ -14,17 +14,13 @@ import { cn } from "@/lib/utils"; import { useTranslations } from "@/lib/i18n/context"; import { DAY_QUERY_PARAM } from "@/lib/day/contract"; import { queryKeys } from "@/lib/query-keys"; -import { randomId } from "@/lib/random-id"; -import { apiDelete, apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; +import { apiDelete, apiGet } from "@/lib/api/api-fetch"; import type { CoachFollowUp, CoachScope } from "@/lib/ai/coach/types"; import type { CoachLaunchScope } from "@/lib/insights/coach-launch-context"; import type { InboundDocumentDetailDto } from "@/lib/validations/inbound-documents"; import { MAX_COACH_ATTACHMENTS } from "@/lib/documents/inbound-constants"; import { useModuleEnabled } from "@/hooks/use-module-enabled"; -import { - parseUploadResponse, - stagedAttachmentPollInterval, -} from "@/components/documents/vault-utils"; +import { stagedAttachmentPollInterval } from "@/components/documents/vault-utils"; import { metricScopeLabelFallback, scopeSourceMetricLabelKey, @@ -33,6 +29,11 @@ import { import { CoachDrawerBody } from "./coach-drawer-body"; import { CoachHero } from "./coach-hero"; import { CoachInput } from "./coach-input"; +import { + uploadCoachAttachment, + useUnmountSignal, + type CoachAttachmentUpload, +} from "./upload-attachment"; import { GuidedQuestionBubble, GuidedSummaryBubble, @@ -554,35 +555,26 @@ export function CoachConversation({ for (const id of ids) attachDoc.mutate(id); } + const unmountSignal = useUnmountSignal(); async function handleUploadNew(files: File[]) { const toUpload = files.slice(0, remainingSlots); for (const file of toUpload) { - const form = new FormData(); - form.append("file", file); - let response: Response; + let upload: CoachAttachmentUpload; try { - response = await apiFetchRaw("/api/documents/inbound", { - method: "POST", - headers: { "Idempotency-Key": randomId() }, - body: form, - }); + upload = await uploadCoachAttachment(file, unmountSignal.current); } catch (failure) { showErrorToast(t("insights.coach.attach.uploadError"), { cause: failure, }); continue; } - const parsed = parseUploadResponse( - response.status, - await response.text(), - ); - if (!parsed.ok) { + if (!upload.result.ok) { showErrorToast(t("insights.coach.attach.uploadError"), { - cause: response, + cause: upload.response, }); continue; } - const doc = parsed.document; + const doc = upload.result.document; // Reflect the new document in the vault + seed its detail so the pill // resolves its title immediately (it indexes in the background). queryClient.invalidateQueries({ diff --git a/src/components/insights/coach-panel/upload-attachment.ts b/src/components/insights/coach-panel/upload-attachment.ts new file mode 100644 index 000000000..972600f0f --- /dev/null +++ b/src/components/insights/coach-panel/upload-attachment.ts @@ -0,0 +1,61 @@ +"use client"; + +/** + * The Coach's "attach a new file" upload: one POST to the document vault, + * with the vault's busy-retry (`upload-busy-retry.ts`), so a file attached + * while the vault is still uploading in another tab waits for a free slot + * instead of failing. + */ +import { useEffect, useRef, type RefObject } from "react"; + +import { apiFetchRaw } from "@/lib/api/api-fetch"; +import { randomId } from "@/lib/random-id"; +import { uploadWithBusyRetry } from "@/components/documents/upload-busy-retry"; +import { + parseUploadResponse, + type UploadResult, +} from "@/components/documents/vault-utils"; + +export interface CoachAttachmentUpload { + result: UploadResult; + /** The last response, for the error toast's cause. */ + response: Response | undefined; +} + +/** Upload one file; a network failure throws, every HTTP answer resolves. */ +export async function uploadCoachAttachment( + file: File, + signal: AbortSignal | undefined, +): Promise { + let response: Response | undefined; + const result = await uploadWithBusyRetry( + async () => { + const form = new FormData(); + form.append("file", file); + const answer = await apiFetchRaw("/api/documents/inbound", { + method: "POST", + headers: { "Idempotency-Key": randomId() }, + body: form, + }); + response = answer; + return parseUploadResponse( + answer.status, + await answer.text(), + answer.headers.get("Retry-After"), + ); + }, + { signal }, + ); + return { result, response }; +} + +/** A signal that aborts when the component unmounts, so no retry timer outlives it. */ +export function useUnmountSignal(): RefObject { + const signal = useRef(undefined); + useEffect(() => { + const controller = new AbortController(); + signal.current = controller.signal; + return () => controller.abort(); + }, []); + return signal; +} diff --git a/src/lib/documents/__tests__/upload-policy.test.ts b/src/lib/documents/__tests__/upload-policy.test.ts index 25c5a82e7..0a804d0d5 100644 --- a/src/lib/documents/__tests__/upload-policy.test.ts +++ b/src/lib/documents/__tests__/upload-policy.test.ts @@ -22,13 +22,13 @@ import { DOCUMENT_DEFAULT_MAX_FILE_BYTES, acquireDocumentUploadSlot, DOCUMENT_UPLOAD_GLOBAL_CONCURRENCY, - DOCUMENT_UPLOAD_PER_USER_CONCURRENCY, DOCUMENT_DEFAULT_QUOTA_BYTES, DOCUMENT_HARD_MAX_FILE_BYTES, detectDocumentType, resolveDocumentLimits, servingClassFor, } from "../upload-policy"; +import { DOCUMENT_UPLOAD_PER_USER_CONCURRENCY } from "../inbound-constants"; // ─── Magic-byte fixtures ───────────────────────────────────────────────────── diff --git a/src/lib/documents/inbound-constants.ts b/src/lib/documents/inbound-constants.ts index 37a2eca47..812928889 100644 --- a/src/lib/documents/inbound-constants.ts +++ b/src/lib/documents/inbound-constants.ts @@ -47,6 +47,13 @@ export function isAiReadSource( export const DOCUMENT_LIST_DEFAULT_LIMIT = 50; +/** + * Uploads one person may have in flight on one server process. The upload + * route refuses the next one with 429 `uploadBusy`; the vault's upload queue + * runs at exactly this width so it never provokes that refusal itself. + */ +export const DOCUMENT_UPLOAD_PER_USER_CONCURRENCY = 2; + /** Max document ids one bulk request may touch. */ export const DOCUMENT_BULK_MAX_IDS = 100; diff --git a/src/lib/documents/upload-policy.ts b/src/lib/documents/upload-policy.ts index 55e114f5a..e8125de2f 100644 --- a/src/lib/documents/upload-policy.ts +++ b/src/lib/documents/upload-policy.ts @@ -30,6 +30,7 @@ */ import { detectAvatarMimeType } from "@/lib/avatar"; import { prisma } from "@/lib/db"; +import { DOCUMENT_UPLOAD_PER_USER_CONCURRENCY } from "@/lib/documents/inbound-constants"; // ─── Limits (§3.3) ────────────────────────────────────────────────────────── @@ -47,9 +48,12 @@ export const DOCUMENT_HARD_MAX_FILE_BYTES = 104_857_600; /** Default per-user quota: 1 GiB. */ export const DOCUMENT_DEFAULT_QUOTA_BYTES = 1_073_741_824; -/** In-memory admission bounds for body accumulation in one server process. */ +/** + * In-memory admission bounds for body accumulation in one server process. The + * per-user bound lives in `inbound-constants.ts` because the web upload queue + * sizes itself from it. + */ export const DOCUMENT_UPLOAD_GLOBAL_CONCURRENCY = 4; -export const DOCUMENT_UPLOAD_PER_USER_CONCURRENCY = 2; let activeDocumentUploads = 0; const activeDocumentUploadsByUser = new Map();