fix(frontend): park uploads that cannot succeed, and stop two false signals
The upload queue gains `parkedFor`, so a photo rejected for a reason that cannot change on its own stops re-pushing itself. A ban used to come back as a generic `forbidden`, which purged the blob and moved the row to `blocked` — a terminal state with no retry button — so lifting a ban restored everything except the photo actually in flight. Ban and release are now distinct codes that keep the blob, charge no attempt, and tell the guest what has to happen. `releaseResolvedParks` drains them at boot from /me/context, because the live `user-shown` / `event-opened` events only reach a tab that was open when the host acted, and the usual sequence is the other way round. Two signals were firing on nothing. A filtered feed set `feedStale` on EVERY delta without deduping — and the delta cursor boundary is inclusive while sse.ts deliberately rewinds `lastEventTime`, so deltas routinely re-return rows already delivered. With the backstop polling every 60-120s, a guest who tapped a hashtag got a "Neue Beiträge" pill they could never clear, each tap costing a full filtered refetch. It now dedupes in both branches. The SSE liveness backstop had the mirror problem: `noteDelivered` harvested id, upload_id AND user_id from every payload, so by the time anything was deleted or anyone banned, their ids were already marked delivered from ordinary traffic about live content. The `deleted_ids` and `hidden_user_ids` clauses were false essentially always, leaving a half-open socket undetected while a host moderated into a feed nobody was listening to. Each event now records only the id its own clause tests. Also: /admin no longer bounces to /join on a cleared session — AUTH_ROUTES had the `/admin` prefix, which suppressed clearAuth() on the dashboard and let the login guard bounce back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -215,6 +215,21 @@ interface QueueEntry {
|
||||
* Cleared by `retryItem` — an explicit tap is the guest changing their mind.
|
||||
*/
|
||||
cancelled?: boolean;
|
||||
/**
|
||||
* Parked waiting for a specific host action, with the blob intact.
|
||||
*
|
||||
* Both values are reversible 403s whose answer cannot change without somebody deciding to
|
||||
* change it, which makes automatic retries pure waste: they re-pushed the whole photo over
|
||||
* cellular on every budget refill for the rest of the night while the guest was told to tap
|
||||
* a camera button that 403s.
|
||||
*
|
||||
* - `'reopen'` — the gallery was released (`gallery_released`). Cleared by `event-opened`.
|
||||
* - `'unban'` — the uploader is banned (`user_banned`). Cleared by `user-shown`.
|
||||
*
|
||||
* An explicit `retryItem` clears either: a deliberate tap is the guest asking us to try
|
||||
* anyway, and if the condition still holds the next response simply re-parks it.
|
||||
*/
|
||||
parkedFor?: 'reopen' | 'unban';
|
||||
blob?: Blob;
|
||||
}
|
||||
|
||||
@@ -266,7 +281,7 @@ onClearAuth(() => queueItems.set([]));
|
||||
let sseBound = false;
|
||||
function bindSse(): void {
|
||||
if (sseBound || typeof window === 'undefined') return;
|
||||
const resume = (options: { resetAttempts?: boolean } = {}) => {
|
||||
const resume = (options: { resetAttempts?: boolean; release?: 'reopen' | 'unban' } = {}) => {
|
||||
void (async () => {
|
||||
await requeueRetriable(options);
|
||||
await processQueue();
|
||||
@@ -275,7 +290,20 @@ function bindSse(): void {
|
||||
// A reopen is a deliberate host action that changes the server's answer, so it's fair to
|
||||
// give parked items a fresh retry budget. A plain reconnect is not — that's the signal
|
||||
// that fires over and over on a flapping AP.
|
||||
onSseEvent('event-opened', () => resume({ resetAttempts: true }));
|
||||
onSseEvent('event-opened', () => resume({ resetAttempts: true, release: 'reopen' }));
|
||||
// An unban is the same kind of evidence, for the guest it names. `user-shown` is broadcast to
|
||||
// everyone (every feed needs to un-hide that user's photos), so check it is actually us
|
||||
// before resuming — otherwise one guest's unban would resume every OTHER banned guest's
|
||||
// queue straight into another 403.
|
||||
onSseEvent('user-shown', (payload) => {
|
||||
let userId: unknown;
|
||||
try {
|
||||
userId = (JSON.parse(String(payload)) as { user_id?: unknown }).user_id;
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
if (userId && userId === getUserId()) resume({ resetAttempts: true, release: 'unban' });
|
||||
});
|
||||
onSseEvent('feed-delta', () => resume());
|
||||
sseBound = true;
|
||||
}
|
||||
@@ -293,8 +321,14 @@ bindSse();
|
||||
*
|
||||
* `resetAttempts` is for signals that are positive evidence the blocking condition changed
|
||||
* (the host reopening the event), where starting the budget over is warranted.
|
||||
*
|
||||
* `release` names the host action that just happened, and un-parks only the items that were
|
||||
* waiting for exactly that (`parkedFor`). A reopen must not resume a banned guest's queue, and
|
||||
* an unban must not resume uploads into a released gallery — both would just 403 again.
|
||||
*/
|
||||
async function requeueRetriable(options: { resetAttempts?: boolean } = {}): Promise<void> {
|
||||
async function requeueRetriable(
|
||||
options: { resetAttempts?: boolean; release?: 'reopen' | 'unban' } = {}
|
||||
): Promise<void> {
|
||||
const myUserId = getUserId();
|
||||
const all = await storeGetAll();
|
||||
const now = Date.now();
|
||||
@@ -306,6 +340,13 @@ async function requeueRetriable(options: { resetAttempts?: boolean } = {}): Prom
|
||||
// not even on `resetAttempts` (the host reopening the event says nothing about whether
|
||||
// this guest still wants this photo sent). Only `retryItem` clears it.
|
||||
if (entry.cancelled) continue;
|
||||
// Parked waiting on a host action. Only the matching signal releases it, so a plain
|
||||
// reconnect leaves it alone instead of re-pushing the photo at a server whose answer
|
||||
// cannot have changed. A manual "Erneut" bypasses this via `retryItem`.
|
||||
if (entry.parkedFor) {
|
||||
if (entry.parkedFor !== options.release) continue;
|
||||
entry.parkedFor = undefined;
|
||||
}
|
||||
if (options.resetAttempts) {
|
||||
entry.attempts = 0;
|
||||
entry.nextAttemptAt = undefined;
|
||||
@@ -612,6 +653,31 @@ class AuthError extends Error {}
|
||||
*/
|
||||
class LockedError extends Error {}
|
||||
|
||||
/**
|
||||
* The gallery has been RELEASED — `gallery_released`. A subclass of `LockedError` so every
|
||||
* blob-preserving code path below keeps treating it as a reversible lock (the host *can* still
|
||||
* reopen, and losing a photo is the worst outcome).
|
||||
*
|
||||
* What differs is the retry policy. A closed event is a pause the host means to undo, so
|
||||
* auto-resuming on reconnect is right. A released gallery is the end of the event, and in the
|
||||
* normal flow nobody reopens it — so auto-retrying re-pushes a multi-megabyte photo over
|
||||
* cellular on every budget refill, forever, for an answer that will not change, while the guest
|
||||
* is told to tap a camera button that 403s. Items parked this way sit still (see
|
||||
* `awaitingReopen` in `requeueRetriable`) until a real `event-opened` arrives or the guest
|
||||
* retries by hand.
|
||||
*/
|
||||
class ReleasedError extends LockedError {}
|
||||
|
||||
/**
|
||||
* The uploader is banned — `user_banned`. Also a `LockedError` subclass, so the blob survives:
|
||||
* `unban_user` exists and the host's confirm copy promises the photos come back, which the old
|
||||
* generic-`forbidden` classification made impossible for anything mid-flight (blob purged, row
|
||||
* moved to `blocked`, and `blocked` has no retry button).
|
||||
*
|
||||
* Parks still like `ReleasedError` and waits for `user-shown`.
|
||||
*/
|
||||
class BannedError extends LockedError {}
|
||||
|
||||
/** Retry policy for an upload response status. */
|
||||
export type UploadOutcome = 'success' | 'rate_limit' | 'auth' | 'transient' | 'terminal';
|
||||
|
||||
@@ -656,11 +722,36 @@ export function classifyUploadStatus(status: number): UploadOutcome {
|
||||
export function isReversibleLock(status: number, errorCode: unknown): boolean {
|
||||
return (
|
||||
errorCode === 'uploads_locked' ||
|
||||
errorCode === 'gallery_released' ||
|
||||
// A ban is lifted by `unban_user`, and the host UI promises the photos come back. Purging
|
||||
// the blob here made that promise impossible to keep for anything mid-flight.
|
||||
errorCode === 'user_banned' ||
|
||||
errorCode === 'quota_exceeded' ||
|
||||
(status === 403 && errorCode !== 'forbidden')
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Within the reversible-lock bucket, is this the END of the event rather than a pause?
|
||||
*
|
||||
* `gallery_released` means the keepsake has been snapshotted. The blob is still kept (a host
|
||||
* reopen is possible), but the item must stop auto-retrying — see `ReleasedError`. Pure +
|
||||
* exported for the same reason as `isReversibleLock`: it decides whether a guest's photo gets
|
||||
* re-pushed over cellular all night.
|
||||
*/
|
||||
export function isGalleryReleased(errorCode: unknown): boolean {
|
||||
return errorCode === 'gallery_released';
|
||||
}
|
||||
|
||||
/**
|
||||
* Is this a ban (`user_banned`)? Reversible, blob kept — but like a release it will not lift on
|
||||
* its own, so the item parks still and waits for the `user-shown` SSE rather than re-pushing the
|
||||
* photo on every reconnect at a guest who is currently not allowed to upload.
|
||||
*/
|
||||
export function isUserBanned(errorCode: unknown): boolean {
|
||||
return errorCode === 'user_banned';
|
||||
}
|
||||
|
||||
/**
|
||||
* Rehydrate a persisted IndexedDB entry into an in-memory `QueueItem`. Pure + exported so the
|
||||
* field-mapping is unit-testable. The rule that must not regress: `lastModified` MUST be carried
|
||||
@@ -716,6 +807,35 @@ export async function loadQueue(): Promise<void> {
|
||||
})();
|
||||
}
|
||||
|
||||
/**
|
||||
* Release parked items whose blocking condition is already over, using the authoritative state
|
||||
* the app fetches at boot.
|
||||
*
|
||||
* `parkedFor` is persisted to IndexedDB, but the only things that cleared it were the LIVE
|
||||
* `event-opened` / `user-shown` SSE events. Those only reach a tab that is open at the moment the
|
||||
* host acts — and the realistic sequence is the opposite one: the guest's photo is parked, they
|
||||
* close the app at the end of the night, and the host lifts the ban or reopens uploads the next
|
||||
* morning. Nothing then ever un-parked the item, so it sat in the queue forever while the toast
|
||||
* had promised "wird gesendet, sobald die Sperre aufgehoben ist".
|
||||
*
|
||||
* Called once per boot with what `/me/context` and the event state actually say, so a park can
|
||||
* never outlive the condition it was waiting on. Cheap: a no-op unless something is parked.
|
||||
*/
|
||||
export async function releaseResolvedParks(state: {
|
||||
banned: boolean;
|
||||
uploadsOpen: boolean;
|
||||
}): Promise<void> {
|
||||
// Each release is scoped to its own signal, exactly as the SSE path is: being unbanned says
|
||||
// nothing about whether the gallery reopened, and vice versa.
|
||||
if (!state.banned) {
|
||||
await requeueRetriable({ resetAttempts: true, release: 'unban' });
|
||||
}
|
||||
if (state.uploadsOpen) {
|
||||
await requeueRetriable({ resetAttempts: true, release: 'reopen' });
|
||||
}
|
||||
await processQueue();
|
||||
}
|
||||
|
||||
/** Outcome of an `addToQueue` call, so the caller can tell the user when a file was NOT
|
||||
* actually queued (deduped, or the queue is full of un-evictable in-flight items). */
|
||||
export type EnqueueResult = 'queued' | 'duplicate' | 'full';
|
||||
@@ -813,6 +933,10 @@ export async function retryItem(id: string): Promise<void> {
|
||||
// And it is the one thing that un-cancels: tapping "Erneut" on a row the guest stopped
|
||||
// themselves is them changing their mind.
|
||||
entry.cancelled = false;
|
||||
// Same for a parked item: an explicit tap is the guest asking us to try anyway (the host may
|
||||
// have reopened or unbanned without this device seeing the SSE). If the condition still
|
||||
// holds the next response re-parks it, so this cannot become a loop.
|
||||
entry.parkedFor = undefined;
|
||||
await storePut(entry);
|
||||
|
||||
queueItems.update((items) =>
|
||||
@@ -1124,7 +1248,16 @@ async function uploadItem(id: string): Promise<void> {
|
||||
// portal) must NOT purge the blob — losing a photo is the worst outcome, and
|
||||
// 403 is the reversible-lock status here.
|
||||
if (isReversibleLock(xhr.status, body?.error)) {
|
||||
settle(() => reject(new LockedError(body?.message || 'Event ist geschlossen.')));
|
||||
const msg = body?.message || 'Event ist geschlossen.';
|
||||
settle(() =>
|
||||
reject(
|
||||
isGalleryReleased(body?.error)
|
||||
? new ReleasedError(msg)
|
||||
: isUserBanned(body?.error)
|
||||
? new BannedError(msg)
|
||||
: new LockedError(msg)
|
||||
)
|
||||
);
|
||||
break;
|
||||
}
|
||||
// Any other 4xx the server will keep rejecting (banned, too large, wrong
|
||||
@@ -1190,20 +1323,31 @@ async function uploadItem(id: string): Promise<void> {
|
||||
// the hourly media reclaim puts them back under it). KEEP the blob and park the item
|
||||
// as retryable so it survives until then. `event-opened` and the `feed-delta`
|
||||
// reconnect both auto-resume it; a manual "Erneut" also works. Never purge here.
|
||||
const exhausted = chargeAttempt(entry);
|
||||
// Neither a release nor a ban lifts on its own, so charging an attempt — and with it
|
||||
// the backoff ladder and budget refill that drive automatic re-pushes — buys nothing
|
||||
// but bandwidth. Park those still and wait for the host action.
|
||||
const parkedFor: 'reopen' | 'unban' | undefined =
|
||||
e instanceof ReleasedError ? 'reopen' : e instanceof BannedError ? 'unban' : undefined;
|
||||
const exhausted = parkedFor ? false : chargeAttempt(entry);
|
||||
entry.status = 'error';
|
||||
entry.error = withRetryHint(e.message, exhausted);
|
||||
entry.parkedFor = parkedFor;
|
||||
entry.error = parkedFor ? e.message : withRetryHint(e.message, exhausted);
|
||||
await storePut(entry);
|
||||
updateItemStatus(id, 'error', entry.error);
|
||||
// Say it out loud. The queue list is only mounted on /upload and the composer sends
|
||||
// the guest straight to /feed, so this message otherwise lands in a store that
|
||||
// nothing on screen renders — the photo just never appears and the guest, with no
|
||||
// operator to ask, assumes it worked.
|
||||
toast(
|
||||
`${entry.fileName}: ${e.message} Du findest den Upload über den Kamera-Button.`,
|
||||
'warning',
|
||||
6000
|
||||
);
|
||||
//
|
||||
// For a release, be explicit that the photo is NOT lost and NOT coming back on its
|
||||
// own — that is the whole difference the guest needs to act on.
|
||||
const parkedHint =
|
||||
parkedFor === 'reopen'
|
||||
? ' Dein Foto bleibt auf diesem Gerät gespeichert — frag die Gastgeber, ob sie die Galerie noch einmal öffnen.'
|
||||
: parkedFor === 'unban'
|
||||
? ' Dein Foto bleibt auf diesem Gerät gespeichert und wird gesendet, sobald die Sperre aufgehoben ist.'
|
||||
: ' Du findest den Upload über den Kamera-Button.';
|
||||
toast(`${entry.fileName}: ${e.message}${parkedHint}`, 'warning', parkedFor ? 9000 : 6000);
|
||||
throw e;
|
||||
}
|
||||
if (e instanceof AuthError) {
|
||||
|
||||
Reference in New Issue
Block a user