Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 2b57f1728e |
@@ -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);
|
||||
|
||||
@@ -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<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'))
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user