diff --git a/electron/media/mediaLinksRegistry.test.ts b/electron/media/mediaLinksRegistry.test.ts index 8d224094b..4247dfb14 100644 --- a/electron/media/mediaLinksRegistry.test.ts +++ b/electron/media/mediaLinksRegistry.test.ts @@ -205,4 +205,90 @@ describe("mediaLinksRegistry", () => { } }); }); + + // `findMediaLinksByFingerprint` is a READ that writes: when the path has + // drifted it refreshes `lastKnownPath` in the background. Not awaiting that + // write is the right call — a lookup should not pay for it — but it means + // nothing is watching the promise, and a rejected promise nobody watches is a + // process-level `unhandledRejection`. Under vitest that fails the run from + // OUTSIDE every test, which is how this was found: 1628 passing tests, a red + // job, and a stack pointing at a temp dir a finished suite had removed. + // + // Both cases below assert the same thing in two ways a real machine produces + // it. Neither asserts that the refresh succeeds — that is precisely what is + // allowed to fail. + describe("the background path refresh", () => { + /** Same bytes at a second path, so a lookup matches by fingerprint and + * then tries to write the new path back. */ + async function registerThenMove(): Promise<{ original: string; moved: string }> { + const original = path.join(tempDir, "moved.webm"); + await writeFileOfSize(original, 900, "m"); + await registerMediaLinks(tempDir, original, { webcamVideoPath: `${original}-cam.webm` }); + const moved = path.join(tempDir, "moved-elsewhere.webm"); + await fs.copyFile(original, moved); + return { original, moved }; + } + + async function withoutUnhandledRejections(fn: () => Promise): Promise { + const rejections: unknown[] = []; + const onRejection = (reason: unknown) => rejections.push(reason); + process.on("unhandledRejection", onRejection); + try { + await fn(); + // Node decides a rejection is unhandled a tick after the microtask + // queue drains, so the assertion needs a real timer, not a flush. + await new Promise((resolve) => setTimeout(resolve, 50)); + } finally { + process.off("unhandledRejection", onRejection); + } + return rejections; + } + + // Running as root defeats the permission bit this case relies on. Skipped + // out loud rather than passing vacuously. + it.skipIf(process.getuid?.() === 0)( + "logs a refresh it cannot write, and still answers the lookup", + async () => { + const { original, moved } = await registerThenMove(); + // Registry readable, directory unwritable: only the write can fail. + await fs.chmod(tempDir, 0o555); + const warned = vi.spyOn(console, "warn").mockImplementation(() => { + // swallowed: the test asserts on it, the suite output does not need it + }); + try { + const rejections = await withoutUnhandledRejections(async () => { + const resolved = await findMediaLinksByFingerprint(tempDir, moved); + // A refresh that failed is not a lookup that failed. + expect(resolved?.webcamVideoPath).toBe(`${original}-cam.webm`); + }); + expect(rejections).toEqual([]); + expect(warned).toHaveBeenCalled(); + } finally { + warned.mockRestore(); + await fs.chmod(tempDir, 0o755); + } + }, + ); + + it("survives the directory disappearing while the refresh is queued", async () => { + // The CI shape: a suite's `afterEach` removes its temp dir while a write + // is still in the queue. Whoever wins the race is fine — what must not + // happen is a rejection escaping into the process. + const { moved } = await registerThenMove(); + const warned = vi.spyOn(console, "warn").mockImplementation(() => { + // may or may not fire: the write is allowed to win the race + }); + try { + const rejections = await withoutUnhandledRejections(async () => { + const lookup = findMediaLinksByFingerprint(tempDir, moved); + await fs.rm(tempDir, { recursive: true, force: true }); + await lookup; + }); + expect(rejections).toEqual([]); + } finally { + warned.mockRestore(); + await fs.mkdir(tempDir, { recursive: true }); + } + }); + }); }); diff --git a/electron/media/mediaLinksRegistry.ts b/electron/media/mediaLinksRegistry.ts index 5fafb9a02..c2507d138 100644 --- a/electron/media/mediaLinksRegistry.ts +++ b/electron/media/mediaLinksRegistry.ts @@ -162,13 +162,21 @@ const writeQueues = new Map>(); function withWriteLock(baseDir: string, fn: () => Promise): Promise { const queue = writeQueues.get(baseDir) ?? Promise.resolve(); const result = queue.then(fn, fn); - writeQueues.set( - baseDir, - result.then( - () => undefined, - () => undefined, - ), + // The tail swallows the outcome so the NEXT writer runs either way; `result` + // keeps the real one for the caller. + const tail = result.then( + () => undefined, + () => undefined, ); + writeQueues.set(baseDir, tail); + // Drop the key once the chain has drained — but only if nothing queued behind + // us in the meantime, or we would strand a tail a later caller is already + // chained on. The map is keyed by an arbitrary directory path, so without + // this it grows for the life of the process; production has one key, a test + // run has one per temp dir. `tail` never rejects, so this cannot leak either. + void tail.then(() => { + if (writeQueues.get(baseDir) === tail) writeQueues.delete(baseDir); + }); return result; } @@ -300,13 +308,24 @@ export async function findMediaLinksByFingerprint( // Path drifted from what's on record — refresh it so the next lookup can // take a cheaper path if one becomes available again. + // + // ponytail: deliberately not awaited — a lookup must not pay for a write it + // does not need — but `void` alone is not fire-and-forget, it is + // fire-and-crash. Nothing was watching this promise, so any failure became an + // unhandled rejection: in the main process that is a process-level event, and + // under vitest it fails the whole run from outside every test (the CI symptom + // was `mkdir ENOENT` when a suite's temp dir was removed while this write was + // still queued, reported after 1628 passing tests). A refresh that cannot + // happen is not worth interrupting anyone over — but it is worth a line. if (match.lastKnownPath !== videoPath) { void updateRegistry(baseDir, (file) => ({ version: 1, entries: file.entries.map((e) => fingerprintsMatch(e.fingerprint, fingerprint) ? { ...e, lastKnownPath: videoPath } : e, ), - })); + })).catch((error) => { + console.warn("[media-links] could not refresh the recorded path:", error); + }); } return {