Adversarial re-review ofc197b2cfound the previous fix both incomplete AND that it introduced a new stuck-state regression. Two independent reviewers flagged the latter. HIGH (data loss, still open after the last fix) — a worker could claim a LIVE seq after a reopen. `claim_job` took any `pending` row. If a reopen landed between `release_gallery`'s spawn and the worker's claim, the row was left pending at the *bumped* seq, so the worker claimed a CURRENT generation and spent minutes exporting a snapshot taken while the event was open and guests were still uploading. On the next re-release (which re-arms `export_released_at` BEFORE it bumps the seq) that worker's finalize and ready-flip both passed their guards, flipped ready on its stale snapshot, and made `enqueue_and_spawn_exports` skip regeneration — silently serving a keepsake missing every upload from the reopen window. The reopen seq-bump did not help: the worker's seq was not stale. Fix: `claim_job` now refuses to claim unless the event is still released. A worker's generation is therefore always born AND finalized inside a single live release epoch. The unclaimed pending row is harmless — the next release resets and re-spawns it. HIGH (regression I introduced inc197b2c) — `open_event`'s event-UPDATE and its export_job seq-bump were two autocommit statements. The first is exactly what re-enables `release_gallery`, so a re-release could slip between them: it spawns a FRESH worker at seq N+1, then our second statement bumps that live generation to N+2, orphaning the worker. Its seq-guarded finalize/fail/flip then all match nothing, the row is stranded `running` with no owner, and the host cannot retry ("bereits freigegeben") — only a process restart recovers it. Fix: both statements now run in one transaction, so the row lock on `event` serializes `release_gallery` behind the commit. LOW - The flip-lost path left its stale archive on disk (asymmetric with the finalize-lost path, which deletes it). If a host reopened and never re-released, no future generation would ever prune it. Now discards `out_path` and still sweeps older gens. - A reopen clears the ready flags server-side, but nothing told the clients: the nav and /export page kept advertising a keepsake that now 404s. Both now refresh on `event-opened` (a superseded worker deliberately emits no `export-progress`). Tests - New: a release broadcasts `export-progress` 100 for both types + one `export-available`. The export SSE had ZERO e2e coverage, and this round made the emit conditional — `/export/status` polling reads the job row, not the SSE, so a regression here would have left every other test green while the live UI silently stopped updating. - New: open ‖ release churn always converges to a consistent, downloadable keepsake. Honestly labelled a STRESS test, not a regression guard: verified it still passes against a deliberately non-transactional build even with ~90 concurrent pairs, so it does NOT cover the atomicity fix (that window is not reproducible out-of-process). - Verified the reopen-supersession test is non-vacuous by empirically reverting the seq-bump — it fails, as intended. Verified: backend 40 tests, svelte-check 0 errors, e2e typecheck clean, e2e 158 passed / 1 skipped on chromium-desktop. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
349 lines
17 KiB
TypeScript
349 lines
17 KiB
TypeScript
/**
|
|
* Regression guard — reopen→re-release export integrity + the stale-keepsake data-loss fix (H1).
|
|
*
|
|
* Two related bugs on this path:
|
|
*
|
|
* (1) Temp-file race: a second worker spawned by a re-release could stomp the first
|
|
* worker's FIXED temp path → a corrupt/truncated keepsake ZIP.
|
|
*
|
|
* (2) Stale keepsake (silent data loss): a worker from the PRIOR release, still streaming a
|
|
* snapshot taken BEFORE a reopen, would finish and unconditionally re-flip
|
|
* `export_*_ready = TRUE` on an archive that predated the reopen — so uploads added
|
|
* during the reopen window were permanently missing from a "ready" keepsake.
|
|
*
|
|
* The fix: a per-(event,type) generation counter `release_seq`, bumped on every (re)release.
|
|
* A worker captures the seq it claimed and writes per-generation temp/final paths
|
|
* (`Gallery.<seq>.zip`); its finalize and ready-flag flip are guarded by `release_seq`, so a
|
|
* superseded worker discards its output instead of resurrecting a stale keepsake. The
|
|
* download follows the current `done` row's `file_path`, never a fixed name.
|
|
*
|
|
* A second, narrower window on the SAME class of bug (fixed alongside): a reopen (`open_event`)
|
|
* landing between a *current* worker's `finalize_job` and its ready-flag flip could resurrect
|
|
* `export_zip_ready=TRUE` on a pre-reopen snapshot, and the next re-release would
|
|
* `if ready { continue }` and skip regeneration. Two layers close it: (1) `open_event` now bumps
|
|
* every `export_job.release_seq`, so a reopen is itself a supersession point — a stale worker's
|
|
* captured seq no longer matches its `release_seq`-guarded flip; (2) the flip UPDATEs are also
|
|
* anchored on `export_released_at IS NOT NULL` as belt-and-suspenders. Layer (1) is what defeats
|
|
* the double-race where a re-release re-arms `export_released_at` before it bumps the seq.
|
|
*
|
|
* Coverage:
|
|
* - churn integrity: rapid reopen→re-release yields exactly one INTACT ZIP, no stuck jobs.
|
|
* - completeness: an upload added during the reopen window IS present in the re-released
|
|
* keepsake (the direct data-loss regression).
|
|
* - supersession (re-release): a re-release SUPERSEDES an in-flight (running) job — it bumps the
|
|
* generation and regenerates, rather than leaving the stale worker to finalize.
|
|
* - supersession (reopen): a reopen ALONE bumps the generation, so a worker holding the
|
|
* pre-reopen seq can never flip a stale ready flag (closes the finalize↔flip double-race
|
|
* deterministically, without depending on sub-millisecond worker timing).
|
|
*/
|
|
import { test, expect } from '../../fixtures/test';
|
|
import { Client } from 'pg';
|
|
import { execFileSync } from 'node:child_process';
|
|
import { writeFileSync, mkdtempSync, readFileSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { join } from 'node:path';
|
|
import { seedUpload } from '../../helpers/seed';
|
|
import { uploadRaw } from '../../helpers/upload-client';
|
|
import { SseListener } from '../../helpers/sse-listener';
|
|
|
|
const BASE = process.env.E2E_FRONTEND_URL ?? 'http://localhost:3101';
|
|
const SLUG = 'e2e-test-event';
|
|
const N_UPLOADS = 6;
|
|
|
|
const PG = {
|
|
host: process.env.E2E_DB_HOST ?? 'localhost',
|
|
port: Number(process.env.E2E_DB_PORT ?? '55432'),
|
|
user: process.env.E2E_DB_USER ?? 'eventsnap_test',
|
|
password: process.env.E2E_DB_PASSWORD ?? 'eventsnap_test',
|
|
database: process.env.E2E_DB_NAME ?? 'eventsnap_test',
|
|
};
|
|
|
|
async function pgQuery<T = any>(sql: string): Promise<T[]> {
|
|
const c = new Client(PG);
|
|
await c.connect();
|
|
try {
|
|
return (await c.query(sql)).rows as T[];
|
|
} finally {
|
|
await c.end();
|
|
}
|
|
}
|
|
|
|
function post(path: string, jwt: string) {
|
|
return fetch(BASE + path, { method: 'POST', headers: { Authorization: `Bearer ${jwt}` } });
|
|
}
|
|
|
|
async function exportStatus(jwt: string): Promise<any> {
|
|
const res = await fetch(BASE + '/api/v1/export/status', { headers: { Authorization: `Bearer ${jwt}` } });
|
|
return res.json();
|
|
}
|
|
|
|
async function mintTicket(jwt: string): Promise<string> {
|
|
const res = await post('/api/v1/export/ticket', jwt);
|
|
return (await res.json()).ticket;
|
|
}
|
|
|
|
async function waitExportDone(jwt: string) {
|
|
await expect
|
|
.poll(async () => {
|
|
const s = await exportStatus(jwt);
|
|
return s.released === true && s.zip?.status === 'done' && s.html?.status === 'done';
|
|
}, { timeout: 60_000, intervals: [500] })
|
|
.toBe(true);
|
|
}
|
|
|
|
/** Download the current ZIP keepsake and return its (non-directory) entry names. */
|
|
async function downloadZipEntries(jwt: string): Promise<string[]> {
|
|
const ticket = await mintTicket(jwt);
|
|
const res = await fetch(BASE + '/api/v1/export/zip?ticket=' + encodeURIComponent(ticket));
|
|
expect(res.status).toBe(200);
|
|
const bytes = Buffer.from(await res.arrayBuffer());
|
|
expect(bytes.subarray(0, 2).toString('latin1')).toBe('PK'); // ZIP magic — not a stomped file
|
|
const dir = mkdtempSync(join(tmpdir(), 'es-zip-'));
|
|
const zipPath = join(dir, 'Gallery.zip');
|
|
writeFileSync(zipPath, bytes);
|
|
execFileSync('unzip', ['-t', zipPath], { stdio: 'pipe' }); // CRC integrity; throws on corruption
|
|
return execFileSync('unzip', ['-Z1', zipPath], { encoding: 'utf8' })
|
|
.split('\n')
|
|
.map((l) => l.trim())
|
|
.filter((l) => l.length > 0 && !l.endsWith('/'));
|
|
}
|
|
|
|
async function zipReleaseSeq(): Promise<number> {
|
|
const [row] = await pgQuery<{ release_seq: string }>(
|
|
`SELECT ej.release_seq FROM export_job ej JOIN event e ON e.id = ej.event_id
|
|
WHERE e.slug = '${SLUG}' AND ej.type = 'zip'`
|
|
);
|
|
return Number(row.release_seq);
|
|
}
|
|
|
|
test.describe('Flow re-review — reopen→re-release export integrity + stale-keepsake (H1)', () => {
|
|
// The real export job runs image processing; give it head-room over the tiny fixtures.
|
|
test.setTimeout(120_000);
|
|
|
|
test('rapid release/reopen/re-release yields exactly one INTACT ZIP and no stuck jobs', async ({
|
|
host,
|
|
}) => {
|
|
// Seed real, decodable uploads while the event is open.
|
|
for (let i = 0; i < N_UPLOADS; i++) {
|
|
await seedUpload(host.jwt, { caption: `pic ${i}` });
|
|
}
|
|
|
|
// First release → export starts and uploads lock (release ⇒ lock).
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
|
|
// Post-release uploads must be rejected (belt-and-suspenders on top of the lock).
|
|
const rejected = await uploadRaw(host.jwt, readFileSync(join(process.cwd(), 'fixtures', 'media', 'sample.jpg')), {
|
|
filename: 'late.jpg',
|
|
contentType: 'image/jpeg',
|
|
});
|
|
expect(rejected.status).toBe(403);
|
|
|
|
// Churn: reopen (clears release + ready flags) then re-release, several times in
|
|
// quick succession. This is the path that used to be able to double-spawn workers
|
|
// over the shared temp file. End on a release so the gallery is left released.
|
|
for (let i = 0; i < 4; i++) {
|
|
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
|
|
const rel = await post('/api/v1/host/gallery/release', host.jwt);
|
|
expect(rel.status).toBe(204);
|
|
}
|
|
|
|
await waitExportDone(host.jwt);
|
|
|
|
// No export_job may be left stuck 'running'.
|
|
const jobs = await pgQuery<{ type: string; status: string }>(
|
|
`SELECT ej.type::text AS type, ej.status::text AS status
|
|
FROM export_job ej JOIN event e ON e.id = ej.event_id
|
|
WHERE e.slug = '${SLUG}'`
|
|
);
|
|
expect(jobs.length).toBe(2);
|
|
for (const j of jobs) expect(j.status, `${j.type} job status`).toBe('done');
|
|
|
|
// Exactly one entry per seeded upload — no entry lost to a stomped temp file, none
|
|
// duplicated by a second worker.
|
|
const listing = await downloadZipEntries(host.jwt);
|
|
expect(listing.length, `zip entries:\n${listing.join('\n')}`).toBe(N_UPLOADS);
|
|
});
|
|
|
|
test('an upload added during the reopen window IS present in the re-released keepsake (H1)', async ({
|
|
host,
|
|
}) => {
|
|
// Release an initial keepsake of 3 uploads.
|
|
for (let i = 0; i < 3; i++) await seedUpload(host.jwt, { caption: `orig ${i}` });
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
await waitExportDone(host.jwt);
|
|
expect((await downloadZipEntries(host.jwt)).length).toBe(3);
|
|
|
|
// Reopen (invalidates the release) → add a 4th upload while unlocked → re-release.
|
|
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
|
|
await seedUpload(host.jwt, { caption: 'added-during-reopen' });
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
await waitExportDone(host.jwt);
|
|
|
|
// The re-released keepsake MUST contain all 4 — the stale-keepsake bug would have left
|
|
// the download at the pre-reopen snapshot of 3, silently dropping the new upload.
|
|
const listing = await downloadZipEntries(host.jwt);
|
|
expect(listing.length, `zip entries:\n${listing.join('\n')}`).toBe(4);
|
|
});
|
|
|
|
test('re-release SUPERSEDES an in-flight (running) export — bumps the generation', async ({
|
|
host,
|
|
}) => {
|
|
// Deterministic proof of the generation guard, independent of export timing. We pin the
|
|
// zip job to `running` with a sentinel to mimic a worker from a prior release that is
|
|
// still mid-export, then drive reopen→re-release.
|
|
//
|
|
// OLD behavior (the bug): the re-enqueue left a `running` row alone, so the stale worker
|
|
// would eventually finalize and re-flip the ready flag on its pre-reopen snapshot.
|
|
// NEW behavior (the fix): the re-enqueue bumps `release_seq` and resets the row, so the
|
|
// stale generation is superseded — its sentinel is wiped and a fresh worker regenerates.
|
|
await seedUpload(host.jwt, { caption: 'a' });
|
|
await seedUpload(host.jwt, { caption: 'b' });
|
|
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
await waitExportDone(host.jwt);
|
|
const seqBefore = await zipReleaseSeq();
|
|
|
|
// Simulate an in-flight worker owning the zip job at the current generation.
|
|
await pgQuery(
|
|
`UPDATE export_job ej SET status = 'running', progress_pct = 77
|
|
FROM event e
|
|
WHERE e.id = ej.event_id AND e.slug = '${SLUG}' AND ej.type = 'zip'`
|
|
);
|
|
|
|
// Reopen (clears release + ready flags AND bumps every export_job's release_seq) then
|
|
// re-release (bumps + resets again). Either bump alone supersedes the pinned running row.
|
|
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
|
|
// The fresh generation must settle to done at a HIGHER release_seq — proving the stale
|
|
// running row was superseded (not left to finalize) and the sentinel is gone.
|
|
await waitExportDone(host.jwt);
|
|
const [row] = await pgQuery<{ status: string; progress_pct: number; release_seq: string }>(
|
|
`SELECT ej.status::text AS status, ej.progress_pct, ej.release_seq
|
|
FROM export_job ej JOIN event e ON e.id = ej.event_id
|
|
WHERE e.slug = '${SLUG}' AND ej.type = 'zip'`
|
|
);
|
|
expect(row.status, 'zip job status').toBe('done');
|
|
expect(Number(row.progress_pct), 'zip job progress').toBe(100);
|
|
expect(Number(row.release_seq), 'release_seq advanced past the superseded generation').toBeGreaterThan(
|
|
seqBefore
|
|
);
|
|
});
|
|
|
|
test('a reopen ALONE bumps release_seq — a pre-reopen worker can never flip a stale ready flag', async ({
|
|
host,
|
|
}) => {
|
|
// Deterministic proof of the reopen-supersession that closes the finalize↔flip double-race.
|
|
// The nasty interleaving (worker finalized `done@S`, THEN reopen, THEN a re-release re-arms
|
|
// `export_released_at` BEFORE it bumps the seq, THEN the stale worker's flip runs and both
|
|
// guards pass) can't be forced by sub-millisecond timing — but its ROOT cause is observable:
|
|
// whether a reopen retires the current generation's seq. If it does, the seq the worker
|
|
// captured (S) is gone for good, so its `release_seq = S`-guarded flip matches nothing no
|
|
// matter when it lands. We assert exactly that invariant.
|
|
await seedUpload(host.jwt, { caption: 'x' });
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
await waitExportDone(host.jwt);
|
|
const seqAtRelease = await zipReleaseSeq();
|
|
|
|
// Reopen only — no re-release. The export_job row is left `done` at the OLD seq by the old
|
|
// code; the fix bumps it here.
|
|
expect((await post('/api/v1/host/event/open', host.jwt)).status).toBe(204);
|
|
|
|
const seqAfterReopen = await zipReleaseSeq();
|
|
expect(
|
|
seqAfterReopen,
|
|
'reopen must bump export_job.release_seq so a pre-reopen worker (seq=' +
|
|
seqAtRelease +
|
|
') is superseded and can never flip a stale ready flag'
|
|
).toBeGreaterThan(seqAtRelease);
|
|
|
|
// And the reopen must have cleared readiness (belt-and-suspenders: the stale worker also
|
|
// can't satisfy `export_released_at IS NOT NULL`).
|
|
const [ev] = await pgQuery<{ zip_ready: boolean; released_at: string | null }>(
|
|
`SELECT export_zip_ready AS zip_ready, export_released_at AS released_at
|
|
FROM event WHERE slug = '${SLUG}'`
|
|
);
|
|
expect(ev.zip_ready, 'reopen clears zip readiness').toBe(false);
|
|
expect(ev.released_at, 'reopen clears the release timestamp').toBeNull();
|
|
});
|
|
|
|
test('open ‖ release churn always converges to a consistent, downloadable keepsake', async ({
|
|
host,
|
|
}) => {
|
|
// Convergence STRESS test, not a regression guard — be honest about what this does and does
|
|
// not prove. `open_event` reopens the event AND bumps every export_job generation, and those
|
|
// two statements must be atomic (they run in one transaction): if they were separate
|
|
// autocommits, a re-release could slip between them, spawn a FRESH worker at seq N+1, and the
|
|
// reopen's second statement would bump that live generation to N+2 — orphaning the worker,
|
|
// stranding its row at `running` forever with the host unable to retry.
|
|
//
|
|
// That interleaving is NOT reproducible from outside the process (verified: this test passes
|
|
// against a deliberately non-transactional build even with ~90 concurrent open/release pairs —
|
|
// the window is microseconds). So it does not guard the atomicity fix; that rests on the
|
|
// transaction being correct by construction. What this DOES pin down is that hammering both
|
|
// endpoints never leaves the export pipeline in a wedged state — no stuck job, no corrupt or
|
|
// missing archive — which is the user-visible property we actually care about.
|
|
await seedUpload(host.jwt, { caption: 'race' });
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
await waitExportDone(host.jwt);
|
|
|
|
// Hammer both endpoints concurrently. The bad window (between open_event's two autocommit
|
|
// statements) is microseconds wide, so a couple of sequential pairs never hit it — we need
|
|
// many in-flight requests contending for pool connections to land a release inside it.
|
|
// Deliberately unchecked: whichever loses a race legitimately 4xx's ("already released" /
|
|
// nothing to reopen). We only care that no interleaving strands a job.
|
|
for (let round = 0; round < 15; round++) {
|
|
const calls: Promise<unknown>[] = [];
|
|
for (let i = 0; i < 6; i++) {
|
|
calls.push(post('/api/v1/host/event/open', host.jwt));
|
|
calls.push(post('/api/v1/host/gallery/release', host.jwt));
|
|
}
|
|
await Promise.all(calls);
|
|
}
|
|
|
|
// Land deterministically in a released state so a fresh generation is definitely in flight.
|
|
await post('/api/v1/host/event/open', host.jwt);
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
|
|
// An orphaned worker leaves its row `running` forever, so this poll would time out.
|
|
await waitExportDone(host.jwt);
|
|
|
|
const jobs = await pgQuery<{ type: string; status: string }>(
|
|
`SELECT ej.type::text AS type, ej.status::text AS status
|
|
FROM export_job ej JOIN event e ON e.id = ej.event_id
|
|
WHERE e.slug = '${SLUG}'`
|
|
);
|
|
expect(jobs.length).toBe(2);
|
|
for (const j of jobs) expect(j.status, `${j.type} job must not be stranded`).toBe('done');
|
|
|
|
// And the keepsake is actually downloadable and intact.
|
|
expect((await downloadZipEntries(host.jwt)).length).toBe(1);
|
|
});
|
|
|
|
test('a release broadcasts export-progress 100 for both types and one export-available', async ({
|
|
host,
|
|
}) => {
|
|
// The ready-flip is now conditional, and a SUPERSEDED worker deliberately emits neither event.
|
|
// That makes the happy path worth pinning: `/export/status` polling (what waitExportDone uses)
|
|
// reads the export_job row, NOT the SSE — so a regression that silently stopped emitting these
|
|
// would leave every other test green while the /host and /export pages stopped updating live.
|
|
const sse = new SseListener();
|
|
await sse.start(host.jwt);
|
|
|
|
await seedUpload(host.jwt, { caption: 'sse' });
|
|
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
|
|
|
for (const type of ['zip', 'html']) {
|
|
await sse.waitForEvent(
|
|
'export-progress',
|
|
(e) => e.data?.type === type && e.data?.progress_pct === 100,
|
|
30_000
|
|
);
|
|
}
|
|
// Fires once BOTH ready flags are set — the authoritative "keepsake is downloadable" signal.
|
|
await sse.waitForEvent('export-available', () => true, 30_000);
|
|
|
|
sse.stop();
|
|
});
|
|
});
|