diff --git a/frontend/src/lib/upload-queue.test.ts b/frontend/src/lib/upload-queue.test.ts index 838c7ad..22c5c88 100644 --- a/frontend/src/lib/upload-queue.test.ts +++ b/frontend/src/lib/upload-queue.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from 'vitest'; import { classifyUploadStatus, + isIncompleteBody, isReversibleLock, entryToQueueItem, shouldAbortForStall, @@ -54,6 +55,59 @@ describe('classifyUploadStatus', () => { * reopen and the photo resumes) or PURGES it (permanent ban / quota). Getting this wrong either * loses a photo the guest expected to survive a reopen, or lets a banned device retry forever. */ +/** + * Regression guard for the data loss this was written for: an iPhone guest on the live event + * got "Error parsing `multipart/form-data` request", the item went terminal, and the ONLY copy + * of the photo was purged from IndexedDB with no retry offered. + * + * The first attempt at the fix keyed on the envelope (`body.error !== 'bad_request'`) and was + * inert, because the backend wraps the multipart error in its own `bad_request` envelope. These + * cases are transcribed from real responses captured against the running backend, so they fail + * if that reasoning is ever reverted. + */ +describe('isIncompleteBody', () => { + const parseError = 'Error parsing `multipart/form-data` request'; + + it('the exact live failure: bad_request envelope carrying the parse error → incomplete', () => { + expect(isIncompleteBody(400, { error: 'bad_request', message: parseError, status: 400 })).toBe( + true + ); + }); + + it('the same error raised mid-file, with the German prefix → incomplete', () => { + expect( + isIncompleteBody(400, { + error: 'bad_request', + message: `Datei konnte nicht gelesen werden: ${parseError}`, + status: 400 + }) + ).toBe(true); + }); + + it('an unparseable body (plain-text rejection, proxy, WAF) → incomplete', () => { + expect(isIncompleteBody(400, null)).toBe(true); + expect(isIncompleteBody(400, undefined)).toBe(true); + }); + + it('a real verdict on the file → NOT incomplete, so it still purges', () => { + expect( + isIncompleteBody(400, { error: 'bad_request', message: 'Datei ist zu groß. Maximum: 500 MB.' }) + ).toBe(false); + expect( + isIncompleteBody(400, { error: 'bad_request', message: 'Keine Datei hochgeladen.' }) + ).toBe(false); + expect( + isIncompleteBody(400, { error: 'bad_request', message: 'Dateityp wird nicht unterstützt.' }) + ).toBe(false); + }); + + it('only applies to 400 — other statuses keep their own rules', () => { + expect(isIncompleteBody(413, { error: 'quota_exceeded' })).toBe(false); + expect(isIncompleteBody(403, null)).toBe(false); + expect(isIncompleteBody(500, null)).toBe(false); + }); +}); + describe('isReversibleLock', () => { it('an `uploads_locked` code is reversible at any status (event closed / released)', () => { expect(isReversibleLock(403, 'uploads_locked')).toBe(true); diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 565756e..511a4ab 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -701,6 +701,48 @@ export function classifyUploadStatus(status: number): UploadOutcome { return 'transient'; } +/** + * Within the `terminal` bucket, is this 400 a TRUNCATED REQUEST rather than a verdict on the + * file? Keep the blob and retry if so. Pure + exported for the same reason as + * `isReversibleLock`: it decides whether a guest keeps their photo. + * + * Keyed on the MESSAGE, not the envelope. The obvious rule — "an app-raised 400 carries + * `bad_request`, so a 400 without it is Axum's plain-text rejection" — does not hold, and was + * verified against the running backend rather than reasoned about: + * + * stream breaks between parts → 400 application/json + * {"error":"bad_request","message":"Error parsing `multipart/…"} + * stream breaks mid-file → 400 application/json, same code, message prefixed + * "Datei konnte nicht gelesen werden: …" + * no boundary in Content-Type → 400 text/plain "Invalid `boundary` for `multipart/…" + * + * Only the third is Axum's own extractor rejection. The first two — the ones a webview or a + * dropping mobile link actually produce — never reach it: the handler pulls the fields itself + * and wraps `MultipartError` in `AppError::BadRequest` (`upload.rs` field loop and chunk loop), + * so they arrive as an ordinary `bad_request` envelope, indistinguishable by code from "file too + * large" or "caption too long". An envelope check therefore never fires for the case this + * exists to catch. Confirmed live: the log line for a real guest failure and for a synthetic + * truncation are byte-identical. + * + * Retrying is safe: nothing was parsed, so nothing was stored and no quota was charged, and + * `X-Client-Upload-Id` makes a duplicate impossible even if the server did see it. + * + * The substring is Axum's `MultipartError` Display text and is therefore an UPSTREAM contract + * this file does not own — an axum upgrade could reword it and silently re-open the data loss. + * The durable fix is a distinct backend code (e.g. `body_incomplete`) that this can prefer once + * it exists; the match is kept as the fallback because it needs no app-image release. + */ +export function isIncompleteBody(status: number, body: unknown): boolean { + if (status !== 400) return false; + const envelope = body as { error?: unknown; message?: unknown } | null | undefined; + // An unparseable body (proxy, WAF, captive portal) cannot be a considered rejection either. + if (!envelope || envelope.error !== 'bad_request') return true; + return ( + typeof envelope.message === 'string' && + envelope.message.includes('Error parsing `multipart/form-data` request') + ); +} + /** * Within the `terminal` bucket, decide whether a 4xx is a REVERSIBLE lock (keep the blob, * park retryable for a host reopen) rather than a permanent rejection (purge the blob). @@ -1252,23 +1294,10 @@ async function uploadItem(id: string): Promise { ); break; case 'terminal': { - // A 400 the APP raised always carries `bad_request` in a JSON envelope - // (too large, wrong type, caption too long, NUL byte). Axum's own multipart - // rejection does not: it is a PLAIN-TEXT 400 ("Error parsing - // `multipart/form-data` request"), so `body` is null here. - // - // That distinction decides whether a guest keeps their photo. An - // unparseable 400 means the request body never arrived intact — a transport - // failure, not a verdict on the file — and it is exactly what an iOS in-app - // browser (WhatsApp) produces when it truncates an XHR upload. Classified as - // terminal, it purged the blob from IndexedDB and offered no retry, so a - // webview hiccup destroyed the only copy the guest had. - // - // Same reasoning the 403 rule below already applies to an unparseable body, - // and safe to retry: nothing was parsed, so nothing was stored and no quota - // was charged — and `X-Client-Upload-Id` makes a duplicate impossible even - // if the server did see it. - if (xhr.status === 400 && body?.error !== 'bad_request') { + // A truncated body is a transport failure, not a verdict on the file, so it + // must not purge the blob. See `isIncompleteBody` for why the envelope alone + // cannot decide this. + if (isIncompleteBody(xhr.status, body)) { settle(() => reject(new NetworkError('Übertragung unvollständig — bitte erneut versuchen')) );