test(frontend): extract CancellableLoader; pin debounce-race invariants in unit tests (0.87.23)
0.87.16 followup. Extract the four debounce-race invariants (predecessor abort, late-result suppression, AbortError swallow, ownership-aware finally) from `loadCoverage` into a reusable `CancellableLoader` helper at $lib/util/cancellable, with 9 unit tests covering each. Page rewrite uses the helper. Also corrects: hooks.server.test.ts (added Node-fetch rejection shape for already-aborted entry; removed redundant tick); admin.test.ts (AbortError-propagation now uses pre-aborted signal + mockRejectedValueOnce mirroring real Node fetch). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.87.22"
|
||||
version = "0.87.23"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.87.22"
|
||||
version = "0.87.23"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mangalord-frontend",
|
||||
"version": "0.87.22",
|
||||
"version": "0.87.23",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
@@ -207,7 +207,7 @@ describe('hooks.server proxy', () => {
|
||||
// 0.87.9 the upstream fetch kept running until the proxy wall-clock
|
||||
// timeout fired.
|
||||
it('chains an already-aborted client signal into the upstream signal immediately', async () => {
|
||||
// Capture the signal the proxy hands fetch; resolve when called.
|
||||
// Capture the signal the proxy hands fetch.
|
||||
let upstream: AbortSignal | undefined;
|
||||
fetchSpy.mockImplementation(async (_url, init) => {
|
||||
upstream = (init as RequestInit).signal ?? undefined;
|
||||
@@ -225,6 +225,26 @@ describe('hooks.server proxy', () => {
|
||||
expect(upstream?.aborted).toBe(true);
|
||||
});
|
||||
|
||||
it('returns 502 when fetch itself rejects with AbortError on an already-aborted signal', async () => {
|
||||
// Production Node fetch rejects synchronously with AbortError if
|
||||
// the signal is already aborted on entry — the prior test mocks
|
||||
// fetch to resolve 200, which leaves the rejection-path
|
||||
// observation uncovered. Mirror Node's real behaviour and
|
||||
// confirm the handler maps the rejection to 502 / upstream_unavailable
|
||||
// (same envelope as any other network failure).
|
||||
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
fetchSpy.mockRejectedValueOnce(new DOMException('aborted', 'AbortError'));
|
||||
const resolve = vi.fn();
|
||||
const ctrl = new AbortController();
|
||||
ctrl.abort();
|
||||
const event = makeEventWithSignal('/api/v1/health', ctrl.signal);
|
||||
const resp = await handle({ event, resolve });
|
||||
expect(resp.status).toBe(502);
|
||||
const body = await resp.json();
|
||||
expect(body.error.code).toBe('upstream_unavailable');
|
||||
expect(errSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('chains a mid-flight client-disconnect into the upstream signal', async () => {
|
||||
// The mid-flight shape: request.signal is live when the handler
|
||||
// starts, then aborts while the upstream fetch is in progress.
|
||||
@@ -232,13 +252,13 @@ describe('hooks.server proxy', () => {
|
||||
let upstream: AbortSignal | undefined;
|
||||
fetchSpy.mockImplementation(async (_url, init) => {
|
||||
upstream = (init as RequestInit).signal ?? undefined;
|
||||
// Let the handler's chaining listener attach, then trip the
|
||||
// client signal — the upstream signal should reflect.
|
||||
// Let the handler's chaining listener attach (it runs on
|
||||
// the next microtask after `handle()` calls fetch), THEN
|
||||
// trip the client signal. `AbortSignal#dispatchEvent` is
|
||||
// synchronous, so the listener runs in-line — no further
|
||||
// tick needed; we observe `upstream.aborted` immediately.
|
||||
await Promise.resolve();
|
||||
clientCtrl.abort();
|
||||
// Tick once so the listener observes the abort before we
|
||||
// check.
|
||||
await Promise.resolve();
|
||||
return new Response('[]', { status: 200 });
|
||||
});
|
||||
const resolve = vi.fn();
|
||||
|
||||
@@ -731,28 +731,19 @@ describe('admin crawler api client', () => {
|
||||
});
|
||||
|
||||
it('getAnalysisMangaCoverage propagates an abort rejection', async () => {
|
||||
// Mid-flight abort: caller flips the AbortController, the fetch
|
||||
// promise rejects, and the api client lets the rejection
|
||||
// surface to its caller (the debounce-race handler treats
|
||||
// AbortError as silent cancellation rather than an error toast).
|
||||
// Production Node fetch rejects synchronously with AbortError
|
||||
// when called with an already-aborted signal. Mirror that
|
||||
// (instead of wiring our own listener and asserting the
|
||||
// rejection we self-fire) so the test exercises the contract
|
||||
// the consumer relies on — the CancellableLoader at
|
||||
// $lib/util/cancellable.ts treats AbortError as `null` and
|
||||
// skips the state update.
|
||||
const ctrl = new AbortController();
|
||||
fetchSpy.mockImplementationOnce(
|
||||
(_url, init) =>
|
||||
new Promise((_resolve, reject) => {
|
||||
const signal = (init as RequestInit).signal;
|
||||
signal?.addEventListener(
|
||||
'abort',
|
||||
() => reject(new DOMException('aborted', 'AbortError')),
|
||||
{ once: true }
|
||||
);
|
||||
})
|
||||
);
|
||||
const inflight = getAnalysisMangaCoverage(
|
||||
{ limit: 25 },
|
||||
{ signal: ctrl.signal }
|
||||
);
|
||||
ctrl.abort();
|
||||
await expect(inflight).rejects.toMatchObject({ name: 'AbortError' });
|
||||
fetchSpy.mockRejectedValueOnce(new DOMException('aborted', 'AbortError'));
|
||||
await expect(
|
||||
getAnalysisMangaCoverage({ limit: 25 }, { signal: ctrl.signal })
|
||||
).rejects.toMatchObject({ name: 'AbortError' });
|
||||
});
|
||||
|
||||
it('getAnalysisChapterCoverage unwraps items', async () => {
|
||||
|
||||
173
frontend/src/lib/util/cancellable.test.ts
Normal file
173
frontend/src/lib/util/cancellable.test.ts
Normal file
@@ -0,0 +1,173 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { CancellableLoader } from './cancellable';
|
||||
|
||||
/**
|
||||
* Helper: a Promise that resolves when `signal` aborts. Mirrors the
|
||||
* shape a real `fetch(url, { signal })` rejects on cancellation.
|
||||
*/
|
||||
function rejectsOnAbort<T = never>(signal: AbortSignal, value?: T): Promise<T> {
|
||||
return new Promise<T>((resolve, reject) => {
|
||||
if (signal.aborted) {
|
||||
reject(new DOMException('aborted', 'AbortError'));
|
||||
return;
|
||||
}
|
||||
signal.addEventListener(
|
||||
'abort',
|
||||
() => reject(new DOMException('aborted', 'AbortError')),
|
||||
{ once: true }
|
||||
);
|
||||
// The caller wires its own resolve via the AbortSignal listeners
|
||||
// in the test; if a value is provided, resolve after a tick.
|
||||
if (value !== undefined) {
|
||||
// microtask so the test can call run() again first.
|
||||
queueMicrotask(() => resolve(value));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
describe('CancellableLoader', () => {
|
||||
it('returns the resolved value when no successor cancels the call', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
const result = await loader.run(async () => 42);
|
||||
expect(result).toBe(42);
|
||||
});
|
||||
|
||||
it('aborts the predecessor signal when a successor starts', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
|
||||
// Predecessor: long-running, observes its own abort.
|
||||
let predecessorSignal: AbortSignal | null = null;
|
||||
const predecessor = loader.run(({ signal }) => {
|
||||
predecessorSignal = signal;
|
||||
return rejectsOnAbort(signal);
|
||||
});
|
||||
|
||||
// Tick the microtask queue so the predecessor's signal is set.
|
||||
await Promise.resolve();
|
||||
expect(predecessorSignal).toBeInstanceOf(AbortSignal);
|
||||
expect(predecessorSignal!.aborted).toBe(false);
|
||||
|
||||
// Successor: resolves immediately.
|
||||
const successor = loader.run(async () => 'B');
|
||||
// The predecessor's signal must have been aborted.
|
||||
expect(predecessorSignal!.aborted).toBe(true);
|
||||
|
||||
// Successor wins; predecessor is swallowed.
|
||||
expect(await successor).toBe('B');
|
||||
expect(await predecessor).toBeNull();
|
||||
});
|
||||
|
||||
it('suppresses a predecessor whose body resolves AFTER the successor started', async () => {
|
||||
// Race: predecessor's fetch hasn't been cancelled in time (e.g.
|
||||
// it had already started writing the response). Its resolved
|
||||
// value must still NOT leak into the loader's caller — the
|
||||
// successor owns the state.
|
||||
const loader = new CancellableLoader();
|
||||
|
||||
// Predecessor: ignores its signal, resolves slowly.
|
||||
let predecessorResolve: (v: string) => void = () => {};
|
||||
const predecessor = loader.run(
|
||||
() =>
|
||||
new Promise<string>((resolve) => {
|
||||
predecessorResolve = resolve;
|
||||
})
|
||||
);
|
||||
await Promise.resolve();
|
||||
|
||||
// Successor: replaces predecessor, resolves quickly.
|
||||
const successor = loader.run(async () => 'fresh');
|
||||
expect(await successor).toBe('fresh');
|
||||
|
||||
// Now resolve the (ignored) predecessor late.
|
||||
predecessorResolve('stale');
|
||||
expect(await predecessor).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null on AbortError without rethrowing', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
// Start a never-resolving call. We can fire its abort by
|
||||
// starting a fresh run() — the AbortError swallow is what
|
||||
// we're pinning here. Crucially, DON'T await `pending` before
|
||||
// starting the successor; the await must come after.
|
||||
const pending = loader.run(({ signal }) => rejectsOnAbort<string>(signal));
|
||||
await Promise.resolve();
|
||||
const successor = loader.run(async () => 'B');
|
||||
expect(await pending).toBeNull();
|
||||
await successor;
|
||||
});
|
||||
|
||||
it('propagates non-AbortError rejections', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
const err = new Error('network kaboom');
|
||||
await expect(loader.run(async () => { throw err; })).rejects.toBe(err);
|
||||
});
|
||||
|
||||
it('isOwnerOf is true only for the current in-flight call', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
// No call in flight: every signal is non-owner.
|
||||
expect(loader.isOwnerOf(new AbortController().signal)).toBe(false);
|
||||
|
||||
let signal: AbortSignal | null = null;
|
||||
const pending = loader.run(({ signal: s }) => {
|
||||
signal = s;
|
||||
return rejectsOnAbort(s);
|
||||
});
|
||||
await Promise.resolve();
|
||||
|
||||
// Mid-flight: the active call's signal is the owner.
|
||||
expect(loader.isOwnerOf(signal!)).toBe(true);
|
||||
// A stranger signal is not.
|
||||
expect(loader.isOwnerOf(new AbortController().signal)).toBe(false);
|
||||
|
||||
// Starting a successor flips ownership.
|
||||
const successor = loader.run(async () => 'next');
|
||||
expect(loader.isOwnerOf(signal!)).toBe(false);
|
||||
await pending;
|
||||
await successor;
|
||||
});
|
||||
|
||||
it('cancel() aborts the current call and leaves the slot empty', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
let signal: AbortSignal | null = null;
|
||||
const pending = loader.run(({ signal: s }) => {
|
||||
signal = s;
|
||||
return rejectsOnAbort(s);
|
||||
});
|
||||
await Promise.resolve();
|
||||
expect(signal!.aborted).toBe(false);
|
||||
|
||||
loader.cancel();
|
||||
expect(signal!.aborted).toBe(true);
|
||||
expect(await pending).toBeNull();
|
||||
// After cancel, isOwnerOf is false for the old signal — slot
|
||||
// empty, no successor queued.
|
||||
expect(loader.isOwnerOf(signal!)).toBe(false);
|
||||
});
|
||||
|
||||
it('rapid burst of three calls: only the last resolves', async () => {
|
||||
// The exact debounce-race shape the dashboard's search box used
|
||||
// to suffer from. Three back-to-back calls; only the third's
|
||||
// value wins; predecessors return null.
|
||||
const loader = new CancellableLoader();
|
||||
|
||||
const a = loader.run(({ signal }) => rejectsOnAbort<string>(signal));
|
||||
await Promise.resolve();
|
||||
const b = loader.run(({ signal }) => rejectsOnAbort<string>(signal));
|
||||
await Promise.resolve();
|
||||
const c = loader.run(async () => 'WIN');
|
||||
|
||||
const [ra, rb, rc] = await Promise.all([a, b, c]);
|
||||
expect(ra).toBeNull();
|
||||
expect(rb).toBeNull();
|
||||
expect(rc).toBe('WIN');
|
||||
});
|
||||
|
||||
it('cleanup: late re-call after a no-cancellation completion finds an empty slot', async () => {
|
||||
const loader = new CancellableLoader();
|
||||
const v = await loader.run(async () => 'first');
|
||||
expect(v).toBe('first');
|
||||
// No predecessor abort fires on the second run; ownership flows.
|
||||
const v2 = await loader.run(async () => 'second');
|
||||
expect(v2).toBe('second');
|
||||
});
|
||||
});
|
||||
95
frontend/src/lib/util/cancellable.ts
Normal file
95
frontend/src/lib/util/cancellable.ts
Normal file
@@ -0,0 +1,95 @@
|
||||
/**
|
||||
* A loader that cancels its in-flight predecessor on each call.
|
||||
*
|
||||
* Background — 0.87.9 added per-loader AbortControllers to the admin
|
||||
* analysis dashboard so a debounced search-burst couldn't race itself
|
||||
* (slow first response overwriting a faster second). The 0.87.16
|
||||
* rereview noted that the contract (predecessor-abort + result-
|
||||
* ownership check + spinner guard) was only inlined into a single
|
||||
* Svelte component and untested.
|
||||
*
|
||||
* Lift the four invariants into a reusable, testable helper:
|
||||
*
|
||||
* 1. **Predecessor abort.** Every `run()` aborts whatever came before
|
||||
* via the prior AbortController.
|
||||
* 2. **Late-result suppression.** A predecessor whose fetch finishes
|
||||
* AFTER the successor's run() started must not surface its value
|
||||
* — `run()` returns `null` ("superseded") on that path so the
|
||||
* caller can skip the state update.
|
||||
* 3. **AbortError swallowed.** Aborts are normal cancellation, not
|
||||
* errors to toast; `run()` returns `null` rather than rethrowing.
|
||||
* 4. **Ownership-aware finally.** Whichever call still owns the
|
||||
* controller clears `this.current` on completion; a later call
|
||||
* that overwrote it leaves the marker alone (or the spinner would
|
||||
* flicker on the older call's abort).
|
||||
*
|
||||
* Usage:
|
||||
* const loader = new CancellableLoader<Result>();
|
||||
* const result = await loader.run(({ signal }) =>
|
||||
* fetch(url, { signal }).then(r => r.json())
|
||||
* );
|
||||
* if (result === null) return; // superseded or aborted
|
||||
* // …apply result…
|
||||
*
|
||||
* Note the `null` outcome is INTENTIONAL — the caller MUST decide
|
||||
* whether to flip its spinner off (yes if no other call is in flight,
|
||||
* no if a successor is). The `isOwnerOf` helper makes that decision
|
||||
* explicit:
|
||||
* if (loader.isOwnerOf(signal)) loading = false;
|
||||
*/
|
||||
export class CancellableLoader {
|
||||
private current: AbortController | null = null;
|
||||
|
||||
/**
|
||||
* Run `fn` with a fresh AbortController, aborting any prior in-
|
||||
* flight call. Returns the resolved value, or `null` when this
|
||||
* call was superseded mid-flight OR fn rejected with AbortError.
|
||||
* Other errors propagate.
|
||||
*/
|
||||
async run<T>(fn: (ctx: { signal: AbortSignal }) => Promise<T>): Promise<T | null> {
|
||||
this.current?.abort();
|
||||
const ctrl = new AbortController();
|
||||
this.current = ctrl;
|
||||
try {
|
||||
const value = await fn({ signal: ctrl.signal });
|
||||
// Successor may have replaced us between the await above
|
||||
// and here. Drop the result silently — the caller should
|
||||
// not write stale data over the successor's state.
|
||||
if (this.current !== ctrl) return null;
|
||||
return value;
|
||||
} catch (e) {
|
||||
// AbortError is normal cancellation; non-Abort errors
|
||||
// (network, 5xx, JSON parse) propagate to the caller.
|
||||
if ((e as { name?: string } | null | undefined)?.name === 'AbortError') {
|
||||
return null;
|
||||
}
|
||||
throw e;
|
||||
} finally {
|
||||
// Only clear the marker if this call still owns it. A
|
||||
// predecessor that lost ownership leaves the successor's
|
||||
// controller untouched.
|
||||
if (this.current === ctrl) this.current = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True iff the given signal belongs to the current in-flight call.
|
||||
* Use from the caller's finally{} to decide whether to flip
|
||||
* loading=false. Reading `signal.aborted` directly works for the
|
||||
* abort case but not for the superseded-without-abort case (only
|
||||
* `null` from `run()` covers both).
|
||||
*/
|
||||
isOwnerOf(signal: AbortSignal): boolean {
|
||||
return this.current?.signal === signal;
|
||||
}
|
||||
|
||||
/**
|
||||
* Abort the current in-flight call without queueing a successor.
|
||||
* Used from onDestroy so a late response can't write into a dead
|
||||
* page.
|
||||
*/
|
||||
cancel(): void {
|
||||
this.current?.abort();
|
||||
this.current = null;
|
||||
}
|
||||
}
|
||||
@@ -17,6 +17,7 @@
|
||||
type AnalysisEvent
|
||||
} from '$lib/api/admin';
|
||||
import { chapterLabel } from '$lib/api/chapters';
|
||||
import { CancellableLoader } from '$lib/util/cancellable';
|
||||
import CoverageBadge from '$lib/components/CoverageBadge.svelte';
|
||||
import Modal from '$lib/components/Modal.svelte';
|
||||
import AnalysisHistoryTable from '$lib/components/analysis/AnalysisHistoryTable.svelte';
|
||||
@@ -71,11 +72,14 @@
|
||||
let live = $state(false);
|
||||
let source: EventSource | null = null;
|
||||
let sseErrors = 0;
|
||||
// Per-loader AbortController so a debounced search-keystroke burst
|
||||
// (or unmount) can cancel its predecessor's response — without this,
|
||||
// a slow first request would overwrite a faster second one, mirroring
|
||||
// the issue the crawler page already solved at line 55.
|
||||
let coverageAbort: AbortController | null = null;
|
||||
// Per-loader cancellation guard. A debounced search-keystroke burst
|
||||
// (or unmount) cancels its predecessor's response — without this,
|
||||
// a slow first request would overwrite a faster second one.
|
||||
// `CancellableLoader` lifts the four invariants (predecessor abort,
|
||||
// late-result suppression, AbortError swallow, ownership-aware
|
||||
// finally) into $lib/util/cancellable, where they're unit-tested.
|
||||
// The inlined version in 0.87.9 was structurally untested.
|
||||
const coverageLoader = new CancellableLoader();
|
||||
|
||||
// Global "now analyzing" pointer, driven by the `started` SSE event so
|
||||
// the operator sees what the worker is on even when nothing is
|
||||
@@ -135,7 +139,7 @@
|
||||
if (clearTimer) clearTimeout(clearTimer);
|
||||
// Cancel any in-flight coverage fetch so a late response can't
|
||||
// overwrite state after the page is gone.
|
||||
coverageAbort?.abort();
|
||||
coverageLoader.cancel();
|
||||
});
|
||||
|
||||
function pushTicker(text: string) {
|
||||
@@ -342,35 +346,36 @@
|
||||
loadingMore = true;
|
||||
}
|
||||
error = null;
|
||||
// Cancel any in-flight predecessor — a debounced search burst
|
||||
// would otherwise race; the slow first response could overwrite
|
||||
// the faster second result.
|
||||
coverageAbort?.abort();
|
||||
const ctrl = new AbortController();
|
||||
coverageAbort = ctrl;
|
||||
try {
|
||||
const r = await getAnalysisMangaCoverage(
|
||||
{
|
||||
search: query.trim() || undefined,
|
||||
limit: LIMIT,
|
||||
offset
|
||||
},
|
||||
{ signal: ctrl.signal }
|
||||
const r = await coverageLoader.run(({ signal }) =>
|
||||
getAnalysisMangaCoverage(
|
||||
{
|
||||
search: query.trim() || undefined,
|
||||
limit: LIMIT,
|
||||
offset
|
||||
},
|
||||
{ signal }
|
||||
)
|
||||
);
|
||||
// `null` = superseded by a successor call, or AbortError.
|
||||
// Skip state updates AND leave the spinner alone — the
|
||||
// successor (which is still in-flight) owns it. Without
|
||||
// this guard, an unconditional `loading = false` in a
|
||||
// finally{} would clear the spinner while the successor
|
||||
// is still loading, producing a flicker.
|
||||
if (r === null) return;
|
||||
mangas = reset ? r.items : [...mangas, ...r.items];
|
||||
total = r.page.total ?? mangas.length;
|
||||
loading = false;
|
||||
loadingMore = false;
|
||||
} catch (e) {
|
||||
// Aborts are normal cancellation, not errors to surface.
|
||||
if ((e as { name?: string })?.name === 'AbortError') return;
|
||||
error = e instanceof ApiError ? e.message : 'Failed to load coverage.';
|
||||
} finally {
|
||||
// Only clear the in-flight markers if this call still owns
|
||||
// them — a later call may have superseded us already.
|
||||
if (coverageAbort === ctrl) coverageAbort = null;
|
||||
if (!ctrl.signal.aborted) {
|
||||
loading = false;
|
||||
loadingMore = false;
|
||||
}
|
||||
// Non-Abort error: this call genuinely failed and OWNS
|
||||
// (any successor would have flipped run() to null before
|
||||
// throwing). Clear the spinner so the error message is
|
||||
// readable.
|
||||
loading = false;
|
||||
loadingMore = false;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user