diff --git a/e2e/specs/02-upload/burst-queue.spec.ts b/e2e/specs/02-upload/burst-queue.spec.ts index fb9762f..a64b773 100644 --- a/e2e/specs/02-upload/burst-queue.spec.ts +++ b/e2e/specs/02-upload/burst-queue.spec.ts @@ -199,9 +199,12 @@ test.describe('Upload — client queue under a burst', () => { // a closed tab / killed PWA. The remaining pending items live only in // IndexedDB now. await page.reload(); - // The queue only resumes where loadQueue() runs — the /upload route's - // onMount. Navigating there is the "reopen the composer" recovery path. - await page.goto('/upload'); + // Deliberately NOT navigating to /upload. Rehydration is now module-level and + // auth-gated (upload-queue.ts `hydrateQueue`), so the queue resumes wherever the + // reload lands. This assertion is the regression guard for the defect it replaced: + // `loadQueue()` used to have a single call site in the whole app — the /upload + // route's onMount — so a guest who reloaded anywhere else saw a 0 badge and their + // staged photos never left the phone, having already been shown a success. // (4) RESUME: every file ends up server-side without re-staging anything. // `>=` not `===`: the only imperfection possible is a DUPLICATE (an upload diff --git a/frontend/src/lib/upload-queue.ts b/frontend/src/lib/upload-queue.ts index 06d396c..18447a7 100644 --- a/frontend/src/lib/upload-queue.ts +++ b/frontend/src/lib/upload-queue.ts @@ -1,6 +1,6 @@ import { openDB, type IDBPDatabase } from 'idb'; import { writable, get } from 'svelte/store'; -import { getToken, getUserId, clearAuth } from './auth'; +import { getToken, getUserId, clearAuth, onSetAuth, onClearAuth } from './auth'; import { onSseEvent } from './sse'; import { refreshQuota } from './quota-store'; import { toast } from './toast-store'; @@ -77,10 +77,7 @@ let onlineBound = false; function bindOnline(): void { if (onlineBound || typeof window === 'undefined') return; window.addEventListener('online', () => { - void (async () => { - await requeueRetriable(); - await processQueue(); - })(); + void loadQueue(); }); onlineBound = true; } @@ -100,10 +97,7 @@ let sseBound = false; function bindSse(): void { if (sseBound || typeof window === 'undefined') return; const resume = () => { - void (async () => { - await requeueRetriable(); - await processQueue(); - })(); + void loadQueue(); }; onSseEvent('event-opened', resume); onSseEvent('feed-delta', resume); @@ -112,28 +106,42 @@ function bindSse(): void { bindSse(); /** - * Flip transient `error` items (5xx / a network drop that got marked before we could - * reclassify it) back to `pending` so a resume actually retries them. Terminal `blocked` - * items (403/413) are left alone — retrying those never succeeds. + * Rebuild the in-memory queue from IndexedDB, flipping transient `error` items (5xx / a + * network drop that got marked before we could reclassify it) back to `pending` so a resume + * actually retries them. Terminal `blocked` items (403/413) are left alone — retrying those + * never succeeds. + * + * REBUILDS rather than patches, and that is the fix. This used to `.map()` over whatever the + * store already held, so it could reset statuses but never ADD an entry — and `processQueue` + * reads only the in-memory store. After a reload or an iOS tab discard the store starts empty, + * so persisted items were invisible to every resume path: the badge read 0 and photos the + * guest had been shown as queued never left the phone. */ async function requeueRetriable(): Promise { const database = await getDb(); const myUserId = getUserId(); const all = await database.getAll(STORE_NAME); - for (const entry of all) { - if (entry.userId === myUserId && entry.status === 'error' && entry.blob) { + // Only surface entries that belong to the current user. Entries from a previous guest on + // this device are filtered out (and are wiped on their next explicit logout via + // `clearQueue`). + const mine = all.filter((entry) => entry.userId && entry.userId === myUserId); + for (const entry of mine) { + if (entry.status === 'error' && entry.blob) { entry.status = 'pending'; entry.error = undefined; await database.put(STORE_NAME, entry); } } - queueItems.update((items) => - items.map((item) => - item.status === 'error' - ? { ...item, status: 'pending' as const, progress: 0, error: undefined } - : item - ) + // Preserve anything actually on the wire. `entryToQueueItem` downgrades `uploading` to + // `pending` with progress 0, so rebuilding blindly would visibly reset the progress bar of + // a request still in flight — and this runs on every `online` event and every SSE + // reconnect, not just at startup. + const inFlight = new Map( + get(queueItems) + .filter((i) => i.status === 'uploading') + .map((i) => [i.id, i]) ); + queueItems.set(mine.map((entry) => inFlight.get(entry.id) ?? entryToQueueItem(entry))); } async function getDb(): Promise { @@ -298,27 +306,55 @@ export function entryToQueueItem(entry: { }; } +/** + * Load the persisted queue into memory and start draining it. + * + * Idempotent and cheap — safe to call from anywhere, as often as you like. + */ export async function loadQueue(): Promise { - const database = await getDb(); - const myUserId = getUserId(); - const all = await database.getAll(STORE_NAME); - // Only surface entries that belong to the current user. Entries from a previous - // guest on this device are filtered out (and would also be wiped on their next - // explicit logout via `clearQueue`). - const items: QueueItem[] = all - .filter((entry) => entry.userId && entry.userId === myUserId) - .map(entryToQueueItem); - queueItems.set(items); - // Staged-but-unsent items from a prior session (queued offline, tab closed before - // reconnect) must resume now — otherwise the "queue flushes when you're back online" - // promise only holds if the user manually re-stages a file. Reclaim transient errors - // (a network drop from a prior session) so they retry instead of stalling. - void (async () => { - await requeueRetriable(); - await processQueue(); - })(); + await requeueRetriable(); + void processQueue(); } +/** + * Rehydrate the queue once per signed-in session, app-wide. + * + * `loadQueue` used to have exactly ONE call site: the `/upload` route's `onMount`. So after a + * reload, an iOS tab discard, or a PWA relaunch, staged photos sat in IndexedDB while the badge + * read 0, and nothing sent them unless the guest happened to navigate back to `/upload` — which + * they have no reason to do, having already been shown a success. + * + * Module level rather than a layout `onMount`, for three reasons: `+layout.svelte` already + * imports this module on every entry point so the side effect runs everywhere with no new + * import; it matches the file's own `bindOnline()` / `bindSse()` pattern directly above; and a + * store module owning its own persistence keeps the layout free of a concern it cannot test. + * + * Re-armed on auth changes because login is a client-side `goto()` — no module re-import and no + * `onMount` re-run — so a hydration that no-oped for lack of a token must get a second chance. + * `auth.ts` does not import this module, so these imports create no cycle. + */ +let hydrated = false; +async function hydrateQueue(): Promise { + if (hydrated || typeof window === 'undefined') return; + if (!getToken() || !getUserId()) return; + hydrated = true; + try { + await loadQueue(); + } catch (e) { + // Let a later signal try again rather than wedging the queue for the session. + hydrated = false; + console.warn('upload queue rehydration failed', e); + } +} +hydrateQueue(); +onSetAuth(() => { + hydrated = false; + void hydrateQueue(); +}); +onClearAuth(() => { + hydrated = false; +}); + /** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT * actually queued (deduped, the queue is full of un-evictable in-flight items, or the * local store itself is unusable). */