mirror of
https://github.com/Donkie/Spoolman.git
synced 2026-10-22 01:52:17 -05:00
client_v2: abort in-flight list queries on navigation
Spamming between the Library and Locations pages queued up a heap of spool queries that kept running after their view was gone: the results were discarded, but the requests still cost bandwidth and backend work, and piled up behind the browser's per-host connection limit. Reads now take an optional AbortSignal (getList/getJson, SpoolQuery and GroupQuery gain a `signal`), and every list view cancels what it no longer needs on effect-cleanup / unmount: FilamentList, GroupRow, PrintLayoutPanel and the locations page — whose MAX_INFLIGHT queue also drops turns still waiting when the page went away. Writes deliberately keep no signal, since a request that has left the browser has already changed the server. isAbortError() takes the signal as the reliable cancellation test, because an aborted fetch does not always reject with a DOMException named AbortError. Measured on a ~4900-spool backend, leaving the Library mid-load went from 17 row queries still completing after navigation to 0 (all cancelled), with no console noise and no stuck cards on return. Claude-Session: https://claude.ai/code/session_014Cwk5Yyze9yPvkYkUtMAP4
This commit is contained in:
1 parent
505faf3b35
commit
a733366be9
9 files changed
+134
-31
No files matched your search
@@ -15,6 +15,22 @@ function queryString(params: QueryParams): string {
|
||||
return s ? '?' + s : '';
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a rejection is the result of cancelling the request rather than a
|
||||
* real failure — callers should return quietly instead of logging it or flagging
|
||||
* the view as broken.
|
||||
*
|
||||
* Pass the signal you cancelled with whenever you have it. It is the reliable
|
||||
* test: an aborted fetch does NOT always reject with a DOMException named
|
||||
* AbortError — Chromium reports a plain `TypeError: Failed to fetch` for aborts
|
||||
* that land while the request is on the wire, which is precisely the case here.
|
||||
* If we asked for the abort, any rejection that follows is ours.
|
||||
*/
|
||||
export function isAbortError(e: unknown, signal?: AbortSignal): boolean {
|
||||
if (signal?.aborted) return true;
|
||||
return e instanceof DOMException && e.name === 'AbortError';
|
||||
}
|
||||
|
||||
/** A non-2xx API response. Carries `status` so callers can report it to the user. */
|
||||
export class HttpError extends Error {
|
||||
constructor(
|
||||
@@ -44,17 +60,30 @@ export interface RawPage {
|
||||
total: number;
|
||||
}
|
||||
|
||||
// Reads take an optional AbortSignal so a view can cancel what it no longer needs
|
||||
// — a superseded query, or everything it had in flight when it was navigated away
|
||||
// from. Writes deliberately don't: a PATCH that has left the browser has already
|
||||
// changed the server, so cancelling it would only hide the result.
|
||||
|
||||
/** GET a list endpoint, returning the parsed array plus the X-Total-Count total. */
|
||||
export async function getList(path: string, params: QueryParams = {}): Promise<RawPage> {
|
||||
const res = await ensureOk(await fetch(API_BASE + path + queryString(params)), 'GET', path);
|
||||
export async function getList(
|
||||
path: string,
|
||||
params: QueryParams = {},
|
||||
signal?: AbortSignal
|
||||
): Promise<RawPage> {
|
||||
const res = await ensureOk(await fetch(API_BASE + path + queryString(params), { signal }), 'GET', path);
|
||||
const items = (await res.json()) as unknown[];
|
||||
const header = res.headers.get('x-total-count');
|
||||
const total = header != null && header !== '' ? Number(header) : items.length;
|
||||
return { items, total: Number.isNaN(total) ? items.length : total };
|
||||
}
|
||||
|
||||
export async function getJson<T = unknown>(path: string, params: QueryParams = {}): Promise<T> {
|
||||
const res = await ensureOk(await fetch(API_BASE + path + queryString(params)), 'GET', path);
|
||||
export async function getJson<T = unknown>(
|
||||
path: string,
|
||||
params: QueryParams = {},
|
||||
signal?: AbortSignal
|
||||
): Promise<T> {
|
||||
const res = await ensureOk(await fetch(API_BASE + path + queryString(params), { signal }), 'GET', path);
|
||||
return (await res.json()) as T;
|
||||
}
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ function currentFilters(state: LibraryState): Record<string, string[]> {
|
||||
}
|
||||
|
||||
/** Page-of-groups query (grouped mode). */
|
||||
export function buildGroupQuery(state: LibraryState): GroupQuery {
|
||||
export function buildGroupQuery(state: LibraryState, signal?: AbortSignal): GroupQuery {
|
||||
const field = state.group as GroupField;
|
||||
const groupField = resolveGroupSortField(state.sortKey) ?? 'group.title';
|
||||
const dir = groupField === 'group.title' ? 'asc' : state.sortAsc ? 'asc' : 'desc';
|
||||
@@ -31,12 +31,18 @@ export function buildGroupQuery(state: LibraryState): GroupQuery {
|
||||
allowArchived: state.showArchived,
|
||||
limit: state.pageSize,
|
||||
offset: (state.page - 1) * state.pageSize,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
};
|
||||
}
|
||||
|
||||
/** Spools of one group, ordered by the chosen Sort. */
|
||||
export function buildScopedSpoolQuery(state: LibraryState, group: GroupSummary, limit: number): SpoolQuery {
|
||||
export function buildScopedSpoolQuery(
|
||||
state: LibraryState,
|
||||
group: GroupSummary,
|
||||
limit: number,
|
||||
signal?: AbortSignal
|
||||
): SpoolQuery {
|
||||
const within = resolveSortField(state.sortKey);
|
||||
const sort: SortField[] = [
|
||||
{ field: within, dir: state.sortAsc ? 'asc' : 'desc' },
|
||||
@@ -49,12 +55,13 @@ export function buildScopedSpoolQuery(state: LibraryState, group: GroupSummary,
|
||||
allowArchived: state.showArchived,
|
||||
limit,
|
||||
offset: 0,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
};
|
||||
}
|
||||
|
||||
/** Flat page-of-spools query (group=none). */
|
||||
export function buildFlatSpoolQuery(state: LibraryState): SpoolQuery {
|
||||
export function buildFlatSpoolQuery(state: LibraryState, signal?: AbortSignal): SpoolQuery {
|
||||
const sort: SortField[] = [
|
||||
{ field: resolveSortField(state.sortKey), dir: state.sortAsc ? 'asc' : 'desc' },
|
||||
{ field: 'id', dir: 'asc' }
|
||||
@@ -65,6 +72,7 @@ export function buildFlatSpoolQuery(state: LibraryState): SpoolQuery {
|
||||
allowArchived: state.showArchived,
|
||||
limit: state.pageSize,
|
||||
offset: (state.page - 1) * state.pageSize,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
};
|
||||
}
|
||||
@@ -11,8 +11,8 @@ export interface SettingResponse {
|
||||
|
||||
export type SettingsMap = Record<string, SettingResponse>;
|
||||
|
||||
export function getSettings(): Promise<SettingsMap> {
|
||||
return getJson<SettingsMap>('/setting/');
|
||||
export function getSettings(signal?: AbortSignal): Promise<SettingsMap> {
|
||||
return getJson<SettingsMap>('/setting/', {}, signal);
|
||||
}
|
||||
|
||||
export async function setSetting(key: string, value: unknown): Promise<void> {
|
||||
|
||||
@@ -126,7 +126,7 @@ class HttpSpoolSource {
|
||||
};
|
||||
applyFilters(params, query.filters);
|
||||
|
||||
const { items, total } = await getList('/spool/group', params);
|
||||
const { items, total } = await getList('/spool/group', params, query.signal);
|
||||
for (const g of items as Json[]) {
|
||||
if (g.filament) {
|
||||
inventory.upsertFilament(mapFilament(g.filament));
|
||||
@@ -147,7 +147,7 @@ class HttpSpoolSource {
|
||||
applyFilters(params, query.filters);
|
||||
scopeParams(params, query.groupScope);
|
||||
|
||||
const { items, total } = await getList('/spool', params);
|
||||
const { items, total } = await getList('/spool', params, query.signal);
|
||||
return { items: cacheSpools(items), total };
|
||||
}
|
||||
|
||||
|
||||
@@ -54,6 +54,8 @@ export interface SpoolQuery {
|
||||
limit: number;
|
||||
offset: number;
|
||||
lowThreshold: number;
|
||||
/** Cancels the request when the view that asked for it is gone or superseded. */
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
export interface GroupQuery {
|
||||
@@ -66,6 +68,8 @@ export interface GroupQuery {
|
||||
limit: number;
|
||||
offset: number;
|
||||
lowThreshold: number;
|
||||
/** Cancels the request when the view that asked for it is gone or superseded. */
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
export interface Page<T> {
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
import { PAPER_NAMES, paperSize, sheetGrid } from '$lib/labels/paper';
|
||||
import { printLabels, saveLabelImages, ZIP_THRESHOLD } from '$lib/labels/print';
|
||||
import { spoolSource } from '$lib/api/spoolSource';
|
||||
import { isAbortError } from '$lib/api/http';
|
||||
import { inventory } from '$lib/stores/inventory.svelte';
|
||||
import { settings } from '$lib/stores/settings.svelte';
|
||||
import type { Spool } from '$lib/types';
|
||||
@@ -48,18 +49,24 @@
|
||||
}
|
||||
|
||||
$effect(() => {
|
||||
void load();
|
||||
const ctrl = new AbortController();
|
||||
void load(ctrl.signal);
|
||||
return () => ctrl.abort();
|
||||
});
|
||||
async function load() {
|
||||
async function load(signal: AbortSignal) {
|
||||
try {
|
||||
await spoolSource.listSpools({
|
||||
filters: {},
|
||||
sort: [{ field: 'id', dir: 'asc' }],
|
||||
limit: 1000,
|
||||
offset: 0,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
});
|
||||
} catch (e) {
|
||||
// This one query is heavy enough that abandoning the tab mid-load is
|
||||
// exactly when cancelling it matters most.
|
||||
if (isAbortError(e, signal)) return;
|
||||
console.error('Failed to load spools for printing', e);
|
||||
} finally {
|
||||
loading = false;
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
import { spoolToVM } from '$lib/utils/library';
|
||||
import { buildGroupQuery, buildFlatSpoolQuery, isGroupedMode } from '$lib/api/query';
|
||||
import { spoolSource } from '$lib/api/spoolSource';
|
||||
import { isAbortError } from '$lib/api/http';
|
||||
import { live } from '$lib/api/live';
|
||||
import * as m from '$lib/paraglide/messages';
|
||||
|
||||
@@ -33,11 +34,16 @@
|
||||
/** Bumped by live events; forces refetch of the page and every GroupRow. */
|
||||
let revision = $state(0);
|
||||
|
||||
// One request at a time: the effect's cleanup aborts the previous one, so a
|
||||
// superseded query (paging, sorting, a live event) — or the whole page being
|
||||
// navigated away from — stops costing bandwidth and backend work instead of
|
||||
// merely having its result discarded.
|
||||
let reqId = 0;
|
||||
$effect(() => {
|
||||
const isGrouped = grouped;
|
||||
const gq = isGrouped ? buildGroupQuery(libraryState) : null;
|
||||
const sq = isGrouped ? null : buildFlatSpoolQuery(libraryState);
|
||||
const ctrl = new AbortController();
|
||||
const gq = isGrouped ? buildGroupQuery(libraryState, ctrl.signal) : null;
|
||||
const sq = isGrouped ? null : buildFlatSpoolQuery(libraryState, ctrl.signal);
|
||||
revision; // refetch on live events
|
||||
const mine = ++reqId;
|
||||
loading = true;
|
||||
@@ -55,11 +61,12 @@
|
||||
total = page.total;
|
||||
loading = false;
|
||||
}).catch((err) => {
|
||||
if (mine !== reqId) return;
|
||||
if (mine !== reqId || isAbortError(err, ctrl.signal)) return;
|
||||
console.error('Failed to load spools', err);
|
||||
errored = true;
|
||||
loading = false;
|
||||
});
|
||||
return () => ctrl.abort();
|
||||
});
|
||||
|
||||
// The list fetches server-paged data outside the reactive cache, so it keeps
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
import { spoolToVM } from '$lib/utils/library';
|
||||
import { buildScopedSpoolQuery } from '$lib/api/query';
|
||||
import { spoolSource } from '$lib/api/spoolSource';
|
||||
import { isAbortError } from '$lib/api/http';
|
||||
import { inventory } from '$lib/stores/inventory.svelte';
|
||||
import { settings } from '$lib/stores/settings.svelte';
|
||||
import { kg } from '$lib/utils/format';
|
||||
@@ -31,14 +32,25 @@
|
||||
// The parent's keyed {#each} recreates this component when the group identity
|
||||
// changes, so `limit` naturally resets; persisting groups keep their window.
|
||||
|
||||
// Every visible group runs one of these, so leaving them in flight after the
|
||||
// list has moved on is what makes rapid paging (or navigating away and back)
|
||||
// pile up queries on the server. The effect cleanup cancels this row's request
|
||||
// whenever it is superseded or the row goes away.
|
||||
let reqId = 0;
|
||||
$effect(() => {
|
||||
const q = buildScopedSpoolQuery(libraryState, group, limit);
|
||||
const ctrl = new AbortController();
|
||||
const q = buildScopedSpoolQuery(libraryState, group, limit, ctrl.signal);
|
||||
revision; // refetch on live events
|
||||
const mine = ++reqId;
|
||||
spoolSource.listSpools(q).then((page) => {
|
||||
if (mine === reqId) spools = page.items;
|
||||
});
|
||||
spoolSource
|
||||
.listSpools(q)
|
||||
.then((page) => {
|
||||
if (mine === reqId) spools = page.items;
|
||||
})
|
||||
.catch((e) => {
|
||||
if (!isAbortError(e, ctrl.signal)) console.error('Failed to load group spools', e);
|
||||
});
|
||||
return () => ctrl.abort();
|
||||
});
|
||||
|
||||
let inUse = $derived(
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
import { settings } from '$lib/stores/settings.svelte';
|
||||
import { spoolSource } from '$lib/api/spoolSource';
|
||||
import { live, type LiveEvent } from '$lib/api/live';
|
||||
import { isAbortError } from '$lib/api/http';
|
||||
import { mapSpool } from '$lib/api/map';
|
||||
import { goto } from '$app/navigation';
|
||||
import { weightAuto } from '$lib/utils/format';
|
||||
@@ -89,7 +90,16 @@
|
||||
|
||||
// The one query that runs up front: every location that currently holds spools,
|
||||
// with its count, aggregated in the database. No spool rows cross the wire.
|
||||
// Everything this page reads is tied to one controller, aborted when the page
|
||||
// goes away. Without it, leaving mid-load leaves a screenful of card queries
|
||||
// running for a server that no longer has anyone to answer — and bouncing in
|
||||
// and out of the page stacks a fresh set on top of each abandoned one.
|
||||
let pageAbort = new AbortController();
|
||||
|
||||
async function loadLocations() {
|
||||
// Captured up front: by the time this settles, `pageAbort` may already be a
|
||||
// fresh controller for a later mount.
|
||||
const signal = pageAbort.signal;
|
||||
try {
|
||||
const page = await spoolSource.listGroups({
|
||||
field: 'location',
|
||||
@@ -97,7 +107,8 @@
|
||||
sort: [{ field: 'group.title', dir: 'asc' }],
|
||||
limit: 1000,
|
||||
offset: 0,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
});
|
||||
// A NULL location and an empty-string one are distinct rows to the database
|
||||
// but the same "No location" card here, so counts are summed by key.
|
||||
@@ -119,6 +130,7 @@
|
||||
b.done = true;
|
||||
}
|
||||
} catch (e) {
|
||||
if (isAbortError(e, signal)) return;
|
||||
console.error('Failed to load locations', e);
|
||||
} finally {
|
||||
locationsLoaded = true;
|
||||
@@ -133,9 +145,20 @@
|
||||
let inflight = 0;
|
||||
const waiting: (() => void)[] = [];
|
||||
|
||||
async function acquire() {
|
||||
/**
|
||||
* Wait for a slot. Returns false if the page was abandoned while queued — the
|
||||
* queue is where most of the waste sits when someone navigates away from a
|
||||
* screenful of cards, so those turns must be dropped rather than sent.
|
||||
*/
|
||||
async function acquire(signal: AbortSignal): Promise<boolean> {
|
||||
if (inflight >= MAX_INFLIGHT) await new Promise<void>((resolve) => waiting.push(resolve));
|
||||
if (signal.aborted) {
|
||||
// Hand the slot straight to the next in line instead of holding it.
|
||||
waiting.shift()?.();
|
||||
return false;
|
||||
}
|
||||
inflight++;
|
||||
return true;
|
||||
}
|
||||
function release() {
|
||||
inflight--;
|
||||
@@ -150,9 +173,14 @@
|
||||
const key = storedKey(name);
|
||||
const b = bucket(key);
|
||||
if (b.loading || b.done) return;
|
||||
const signal = pageAbort.signal;
|
||||
b.started = true;
|
||||
b.loading = true;
|
||||
await acquire();
|
||||
if (!(await acquire(signal))) {
|
||||
b.loading = false;
|
||||
b.started = false;
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const page = await spoolSource.listSpools({
|
||||
filters: {},
|
||||
@@ -160,7 +188,8 @@
|
||||
groupScope: { field: 'location', key },
|
||||
limit: PAGE,
|
||||
offset: b.spools.length,
|
||||
lowThreshold: settings.lowThreshold
|
||||
lowThreshold: settings.lowThreshold,
|
||||
signal
|
||||
});
|
||||
// A spool moved in by a drag or a live event may already be held here.
|
||||
const seen = new Set(b.spools.map((s) => s.id));
|
||||
@@ -170,8 +199,9 @@
|
||||
b.done = page.items.length === 0 || b.spools.length >= page.total;
|
||||
} catch (e) {
|
||||
// Leave the card unstarted so scrolling it back into view retries, rather
|
||||
// than stranding it empty for the rest of the session.
|
||||
console.error('Failed to load spools for location', name, e);
|
||||
// than stranding it empty for the rest of the session. A cancelled request
|
||||
// is the same story without the noise.
|
||||
if (!isAbortError(e, signal)) console.error('Failed to load spools for location', name, e);
|
||||
b.started = false;
|
||||
} finally {
|
||||
release();
|
||||
@@ -208,11 +238,13 @@
|
||||
}
|
||||
|
||||
async function loadLocationsSetting() {
|
||||
const signal = pageAbort.signal;
|
||||
try {
|
||||
const s = await getSettings();
|
||||
const s = await getSettings(signal);
|
||||
settingOrder = parseSetting<string[]>(s.locations, []).filter((l) => l != null);
|
||||
spoolOrders = parseSetting<Record<string, number[]>>(s.locations_spoolorders, {});
|
||||
} catch (e) {
|
||||
if (isAbortError(e, signal)) return;
|
||||
console.error('Failed to load locations setting', e);
|
||||
} finally {
|
||||
settingsLoaded = true;
|
||||
@@ -270,12 +302,16 @@
|
||||
}
|
||||
|
||||
$effect(() => {
|
||||
pageAbort = new AbortController();
|
||||
loadLocations();
|
||||
loadLocationsSetting();
|
||||
const off = live.subscribe('spool', {}, applyLiveEvent);
|
||||
return () => {
|
||||
off();
|
||||
if (countsTimer) clearTimeout(countsTimer);
|
||||
// Drops both the requests already on the wire and the ones still queued
|
||||
// behind MAX_INFLIGHT.
|
||||
pageAbort.abort();
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user