fabi/EventSnap
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(upload-queue): rehydrate the persisted queue app-wide, not only on /upload

Browse Source 
loadQueue() had exactly one call site in the entire frontend: 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. The photo never leaves the phone and the guest is
never told.

The root cause was narrower than "loadQueue isn't called enough".
requeueRetriable() read IndexedDB but only .map()'d over whatever the in-memory
store already held, so it could reset statuses and never ADD an entry — and
processQueue reads only that store. That is why the `online` listener and the SSE
resume hooks, which both call it, could not recover a cold start either. It now
REBUILDS the store from IndexedDB, which makes all three resume paths work.

Rebuilding needs one guard: entryToQueueItem downgrades `uploading` to `pending`
with progress 0, and this runs on every `online` event and every SSE reconnect,
so a blind rebuild would visibly reset the progress bar of a request still on the
wire. In-flight items are carried over by id.

Hydration is module-level, SSR-guarded and idempotent, re-armed via
onSetAuth/onClearAuth because login is a client-side goto() — no module
re-import, no onMount re-run — so a hydration that no-oped for lack of a token
gets a second chance. Module level rather than a layout onMount because
+layout.svelte already imports this module on every entry point, it matches the
file's own bindOnline()/bindSse() pattern, and a store owning its own persistence
keeps the layout free of a concern it cannot test. auth.ts does not import this
module, so no cycle.

The burst-queue e2e test no longer navigates to /upload after its reload — it now
asserts the resume happens wherever the reload lands, which is the actual
regression guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-31 22:58:26 +02:00
parent e52b2f1cd1
commit 3f0f9c098b
2 changed files with 80 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
e2e/specs/02-upload/burst-queue.spec.ts
View File

@@ -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

112
frontend/src/lib/upload-queue.ts
View File

@@ -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<void> {
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<IDBPDatabase> {
@@ -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<void> {
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<void> {
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). */