2 Commits

Author SHA1 Message Date
0fba8defc2 fix(upload): iPhone sent an EMPTY body -- the queue stored a file reference, not bytes
Some checks failed
Audit / cargo audit (backend) (push) Failing after 9m1s
Audit / npm audit (frontend) (push) Successful in 1m10s
Checks / Backend — cargo test + clippy + fmt (push) Failing after 55s
Checks / Keepsake viewer — builds, self-contained, committed artifact in sync (push) Has been cancelled
Checks / E2E — typecheck + lint (push) Has been cancelled
E2E / Playwright E2E (chromium + webkit) (push) Has been cancelled
E2E / Cross-UA smoke matrix (push) Has been cancelled
Checks / Frontend — vitest + svelte-check (push) Has been cancelled
Measured at the reverse proxy during the live event, not inferred:

  status=201 content_length=6449056 dur=3.473s dev=Android
  status=400 content_length=0       dur=0.022s dev=iPhone
  status=400 content_length=0       dur=0.008s dev=iPhone
  status=400 content_length=0       dur=0.009s dev=iPhone

Content-Length ZERO, in 7-22 ms. Nothing was ever put on the wire, which is why
this was never a transport problem -- HTTP/3 was disabled first on the theory
that QUIC was truncating large POSTs, and it changed nothing.

`addToQueue` stored the picked `File` itself: `blob: file`. WebKit persists a
File in IndexedDB as a REFERENCE to the OS backing file rather than a copy, and
iOS deletes that file soon after the picker closes. What is left is a neutered
File: `.name` and `.size` still read correctly, so nothing downstream looks
wrong, and `xhr.send()` does NOT throw -- the note at the send site assumed it
would -- it sends an empty body. The server cannot parse a multipart with no
parts and answers 400 "Error parsing `multipart/form-data` request", which is
the same 400 a genuinely truncated upload produces.

That collision is what made v0.18.4 make things worse: it reclassified that 400
as retryable, so each dead photo re-sent an empty body five times. 79 failed
requests, 1 success, four guests with nothing uploaded.

TWO FIXES.

Bytes are copied at pick time, so IndexedDB owns data no OS purge can reach.
Chunked at 4 MB rather than one `arrayBuffer()`: this queue accepts videos up to
500 MB and pulling that into the JS heap would get the tab killed, trading a
failed upload for a crash. Each chunk becomes its own Blob, so peak heap is one
chunk and the browser's blob store holds the rest, spilling to disk as it sees
fit.

An unreadable blob is TERMINAL, never retried. `entry.blob.size` cannot detect
it -- a neutered File reports the original size -- so the check probes one real
byte before send. Items queued before this fix are still sitting in IndexedDB
holding dead references, and this is what stops them retrying forever. The
message asks the guest to re-pick the photo, because the photo is fine: it is
still in the camera roll, only the browser's copy is gone.

The batch keeps draining past one of these: other queued photos may be readable.
2026-08-22 15:54:06 +00:00
2b57f1728e fix(upload): the truncation guard never fired -- it checked the wrong field
Some checks failed
Audit / cargo audit (backend) (push) Failing after 9m29s
Audit / npm audit (frontend) (push) Successful in 59s
Checks / Backend — cargo test + clippy + fmt (push) Failing after 59s
Checks / Frontend — vitest + svelte-check (push) Failing after 5m40s
Checks / Keepsake viewer — builds, self-contained, committed artifact in sync (push) Failing after 5m18s
Checks / E2E — typecheck + lint (push) Failing after 37s
E2E / Playwright E2E (chromium + webkit) (push) Failing after 8m55s
E2E / Cross-UA smoke matrix (push) Failing after 4m2s
v0.18.3 added a 400 branch to keep a guest's photo when the request body arrives
truncated, gated on `body.error !== 'bad_request'`. Its premise was that axum's
multipart rejection is a plain-text 400 with no envelope, so an envelope with
`bad_request` in it must be a considered verdict on the file.

That is false for the path that actually fails, and the branch was inert against
the exact incident it was written for. A guest on the live event lost a photo at
07:22; replaying the same truncation against the running backend at 07:50
produced a byte-identical log line:

  WARN request rejected status=400 code="bad_request"
       detail="Error parsing `multipart/form-data` request"

and this response body:

  400 application/json
  {"error":"bad_request","message":"Error parsing `multipart/form-data` request"}

A `bad_request` envelope. The guard evaluates false, the item still goes
terminal, and the blob is still purged.

The handler never reaches axum's extractor rejection: it pulls the fields itself
and wraps `MultipartError` in `AppError::BadRequest` -- bare from the field loop,
prefixed with "Datei konnte nicht gelesen werden: " from the chunk loop. Both
arrive indistinguishable BY CODE from "file too large". Axum's own plain-text
rejection does exist (no boundary in Content-Type -> "Invalid `boundary` ...")
but is a different error and not one a webview produces.

So the rule keys on the MESSAGE, and moves into an exported `isIncompleteBody`
beside `isReversibleLock`, matching how the other data-loss-critical rules in
this file are made testable. Tests transcribe the four responses captured from
the running backend, so reverting to an envelope check fails them.

Matching an upstream Display string is the weakness here and is called out in
the doc comment: an axum upgrade could reword it and silently re-open the data
loss. The durable fix is a distinct backend code (`body_incomplete`) this can
prefer once it exists -- deliberately not done now, because it means an app image
release and the backend has not needed one since v0.18.0.
2026-08-22 08:13:40 +00:00
2 changed files with 224 additions and 19 deletions

View File

@@ -1,6 +1,7 @@
import { describe, it, expect } from 'vitest';
import {
classifyUploadStatus,
isIncompleteBody,
isReversibleLock,
entryToQueueItem,
shouldAbortForStall,
@@ -54,6 +55,91 @@ 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.
*/
/**
* The live-event failure this exists to prevent, recorded so it cannot be reintroduced.
*
* iPhone Safari sent POSTs to /api/v1/upload with Content-Length: 0 in 7-22 ms — measured at
* the reverse proxy, alongside an Android upload of 6,449,056 bytes that returned 201. Cause:
* `addToQueue` stored the picked `File` in IndexedDB, and WebKit persists that as a reference
* to an OS file which iOS then deletes. The File keeps its name and size and reads as nothing,
* and `xhr.send()` does not throw — it puts an empty body on the wire.
*
* Two rules follow, and both are asserted by the behaviour under test elsewhere in this file:
* 1. bytes are copied at pick time, so IndexedDB owns data rather than a file reference;
* 2. an unreadable blob is TERMINAL, never retried — retrying an empty body produced 79
* failed requests during the event and could never have succeeded.
*
* Rule 2 is the one with a pure predicate to pin: a 400 whose body carries the multipart parse
* error is only retryable when the request actually had bytes in it. `isIncompleteBody`
* classifies the RESPONSE; the emptiness check happens before send and short-circuits it.
*/
describe('empty-body regression (iPhone neutered File)', () => {
it('the server response to an empty body still looks like a truncation', () => {
// Same 400 either way — which is exactly why the client must not rely on the response
// to tell a truncated upload from one that never had bytes. The pre-send readability
// probe is what separates them.
expect(
isIncompleteBody(400, {
error: 'bad_request',
message: 'Error parsing `multipart/form-data` request'
})
).toBe(true);
});
});
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);

View File

@@ -628,6 +628,14 @@ class TerminalError extends Error {
*/
class NetworkError extends Error {}
/**
* The blob is in IndexedDB but its bytes are unreadable — iOS purged the OS file behind a
* stored `File`. Deliberately NOT a NetworkError: retrying cannot bring the bytes back, and
* treating it as transient is what produced an empty-POST retry storm during the event. The
* guest has to re-pick the photo, and the message says so.
*/
class UnreadableBlobError extends Error {}
/**
* The guest aborted this upload themselves (the ✕ on an in-flight row). A NetworkError
* subclass because the transport outcome is identical — but it must NOT stop the batch or
@@ -701,6 +709,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).
@@ -841,6 +891,33 @@ export async function releaseResolvedParks(state: {
* actually queued (deduped, or the queue is full of un-evictable in-flight items). */
export type EnqueueResult = 'queued' | 'duplicate' | 'full';
/** Chunk size for `materialise`. Bounds peak JS heap, not total copy size. */
const MATERIALISE_CHUNK_BYTES = 4 * 1024 * 1024;
/**
* Copy a picked file's bytes into a Blob this origin owns, so IndexedDB stores DATA rather
* than a reference to an OS file that iOS will delete. See the call site in `addToQueue` for
* why that reference is the bug.
*
* Chunked deliberately. `new Blob([await file.arrayBuffer()])` is one line and correct for a
* 3 MB photo, but it pulls the whole file into the JS heap — and this queue accepts videos up
* to 500 MB, where that would very likely get the tab killed by the OS. Trading a crash for a
* failed upload is not a fix. Reading a slice at a time and letting each chunk become its own
* Blob keeps peak heap at one chunk; the browser's blob store owns the accumulated parts and
* can spill them to disk, which is exactly where a half-gigabyte video should live.
*/
async function materialise(file: File): Promise<Blob> {
if (file.size <= MATERIALISE_CHUNK_BYTES) {
return new Blob([await file.arrayBuffer()], { type: file.type });
}
const parts: Blob[] = [];
for (let offset = 0; offset < file.size; offset += MATERIALISE_CHUNK_BYTES) {
const slice = file.slice(offset, offset + MATERIALISE_CHUNK_BYTES);
parts.push(new Blob([await slice.arrayBuffer()]));
}
return new Blob(parts, { type: file.type });
}
export async function addToQueue(
file: File,
caption: string,
@@ -887,6 +964,23 @@ export async function addToQueue(
// This id is also the server-side idempotency key (`client_upload_id`), so it is minted
// exactly ONCE per file here and reused by every retry — see uploadItem.
const id = uuid();
// MATERIALISE THE BYTES. Do not store the `File` itself.
//
// WebKit persists a File in IndexedDB as a REFERENCE to the OS backing file rather than a
// copy of its contents. iOS purges that file soon after the picker closes, which leaves a
// "neutered File": `.name` and `.size` still read correctly, so nothing looks wrong, but
// the bytes are gone. WebKit then does NOT throw on `xhr.send()` — the note at the send
// site assumed it would — it puts the request on the wire with an EMPTY BODY, the server
// cannot parse a multipart with no parts, and the guest sees a 400.
//
// Measured on the live event rather than inferred: every failing iPhone upload reached
// Caddy with `Content-Length: 0` in 7-22 ms, while an Android upload in the same minute
// sent 6,449,056 bytes and got a 201.
//
// Reading the file here makes IndexedDB own real bytes that no OS purge can reach. It
// costs one full read at pick time, which is also the moment the file is guaranteed still
// readable — the picker has only just handed it over.
const blob = await materialise(file);
const entry: QueueEntry = {
id,
userId,
@@ -897,7 +991,7 @@ export async function addToQueue(
caption,
hashtags,
status: 'pending',
blob: file
blob
};
await storePut(entry);
@@ -1070,6 +1164,11 @@ async function processQueue(): Promise<void> {
// NetworkError, which it extends.)
continue;
}
if (e instanceof UnreadableBlobError) {
// This one photo is unrecoverable, but the others in the queue may be fine
// (a re-picked copy, or one taken after the fix). Keep draining.
continue;
}
if (e instanceof NetworkError) {
// Connectivity dropped mid-flight. If offline the item is back to 'pending'
// and the `online` listener resumes it; if the failure hit while nominally
@@ -1120,7 +1219,26 @@ async function uploadItem(id: string): Promise<void> {
// and charging the guest's quota twice. Both 200 (deduped) and 201 (created) are
// success; `classifyUploadStatus` already treats the whole 2xx range that way.
formData.append('client_upload_id', entry.id);
formData.append('file', entry.blob, entry.fileName);
// Never send a body we cannot read. `entry.blob.size` is NOT sufficient on WebKit: a
// neutered File keeps its metadata and reports the original size while reading as
// nothing. Only an actual read tells the truth, so probe one byte.
//
// This covers items queued BEFORE the materialise-on-pick fix above, which are still
// sitting in IndexedDB holding a dead File reference. Without it those retry until the
// budget is spent, every attempt an empty POST — 79 of them during the event.
const blob = entry.blob;
let readable = false;
try {
readable = (await blob.slice(0, 1).arrayBuffer()).byteLength > 0;
} catch {
readable = false;
}
if (!readable && entry.fileSize > 0) {
throw new UnreadableBlobError(
'Dieses Foto ist auf dem Gerät nicht mehr lesbar — bitte wähle es noch einmal aus.'
);
}
formData.append('file', blob, entry.fileName);
if (entry.caption) formData.append('caption', entry.caption);
if (entry.hashtags) formData.append('hashtags', entry.hashtags);
@@ -1252,23 +1370,10 @@ async function uploadItem(id: string): Promise<void> {
);
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'))
);
@@ -1451,6 +1556,20 @@ async function uploadItem(id: string): Promise<void> {
}
throw e;
}
if (e instanceof UnreadableBlobError) {
// The bytes are gone from the browser's storage (iOS purged the OS file behind a
// stored `File`). No retry can recover them, so this is terminal — but unlike a
// server rejection the photo itself is fine and still in the camera roll, so the
// message asks for a re-pick rather than reporting the file as refused. Dropping
// the dead blob also frees the queue slot for the re-picked copy.
delete entry.blob;
entry.status = 'blocked';
entry.error = e.message;
await storePut(entry);
updateItemStatus(id, 'blocked', e.message);
toast(`${entry.fileName}: ${e.message}`, 'error', 8000);
throw e;
}
if (e instanceof TerminalError) {
// Permanent rejection — drop the blob (we'll never resend it) and mark blocked
// so the UI shows a clear reason and offers no retry.