From a733366be98a9547531a3be4ebe7afd07d3e9923 Mon Sep 17 00:00:00 2001 From: Donkie Date: Mon, 20 Jul 2026 08:01:40 +0200 Subject: [PATCH] client_v2: abort in-flight list queries on navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- client_v2/src/lib/api/http.ts | 37 ++++++++++++-- client_v2/src/lib/api/query.ts | 20 +++++--- client_v2/src/lib/api/settings.ts | 4 +- client_v2/src/lib/api/spoolSource.ts | 4 +- client_v2/src/lib/api/types.ts | 4 ++ .../components/labels/PrintLayoutPanel.svelte | 13 +++-- .../components/library/FilamentList.svelte | 13 +++-- .../lib/components/library/GroupRow.svelte | 20 ++++++-- client_v2/src/routes/locations/+page.svelte | 50 ++++++++++++++++--- 9 files changed, 134 insertions(+), 31 deletions(-) diff --git a/client_v2/src/lib/api/http.ts b/client_v2/src/lib/api/http.ts index 80823145..d911a110 100644 --- a/client_v2/src/lib/api/http.ts +++ b/client_v2/src/lib/api/http.ts @@ -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 { - const res = await ensureOk(await fetch(API_BASE + path + queryString(params)), 'GET', path); +export async function getList( + path: string, + params: QueryParams = {}, + signal?: AbortSignal +): Promise { + 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(path: string, params: QueryParams = {}): Promise { - const res = await ensureOk(await fetch(API_BASE + path + queryString(params)), 'GET', path); +export async function getJson( + path: string, + params: QueryParams = {}, + signal?: AbortSignal +): Promise { + const res = await ensureOk(await fetch(API_BASE + path + queryString(params), { signal }), 'GET', path); return (await res.json()) as T; } diff --git a/client_v2/src/lib/api/query.ts b/client_v2/src/lib/api/query.ts index cfdd4604..4981497a 100644 --- a/client_v2/src/lib/api/query.ts +++ b/client_v2/src/lib/api/query.ts @@ -20,7 +20,7 @@ function currentFilters(state: LibraryState): Record { } /** 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 }; } diff --git a/client_v2/src/lib/api/settings.ts b/client_v2/src/lib/api/settings.ts index 8d0a1c2b..441b7b36 100644 --- a/client_v2/src/lib/api/settings.ts +++ b/client_v2/src/lib/api/settings.ts @@ -11,8 +11,8 @@ export interface SettingResponse { export type SettingsMap = Record; -export function getSettings(): Promise { - return getJson('/setting/'); +export function getSettings(signal?: AbortSignal): Promise { + return getJson('/setting/', {}, signal); } export async function setSetting(key: string, value: unknown): Promise { diff --git a/client_v2/src/lib/api/spoolSource.ts b/client_v2/src/lib/api/spoolSource.ts index 9e67e90f..0b7a4c59 100644 --- a/client_v2/src/lib/api/spoolSource.ts +++ b/client_v2/src/lib/api/spoolSource.ts @@ -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 }; } diff --git a/client_v2/src/lib/api/types.ts b/client_v2/src/lib/api/types.ts index ec9d6f78..521d45c0 100644 --- a/client_v2/src/lib/api/types.ts +++ b/client_v2/src/lib/api/types.ts @@ -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 { diff --git a/client_v2/src/lib/components/labels/PrintLayoutPanel.svelte b/client_v2/src/lib/components/labels/PrintLayoutPanel.svelte index 1312cd6d..59d22622 100644 --- a/client_v2/src/lib/components/labels/PrintLayoutPanel.svelte +++ b/client_v2/src/lib/components/labels/PrintLayoutPanel.svelte @@ -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; diff --git a/client_v2/src/lib/components/library/FilamentList.svelte b/client_v2/src/lib/components/library/FilamentList.svelte index 40760242..adcf8a9c 100644 --- a/client_v2/src/lib/components/library/FilamentList.svelte +++ b/client_v2/src/lib/components/library/FilamentList.svelte @@ -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 diff --git a/client_v2/src/lib/components/library/GroupRow.svelte b/client_v2/src/lib/components/library/GroupRow.svelte index 7daf7675..06cdce9d 100644 --- a/client_v2/src/lib/components/library/GroupRow.svelte +++ b/client_v2/src/lib/components/library/GroupRow.svelte @@ -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( diff --git a/client_v2/src/routes/locations/+page.svelte b/client_v2/src/routes/locations/+page.svelte index 2535c737..a66a4565 100644 --- a/client_v2/src/routes/locations/+page.svelte +++ b/client_v2/src/routes/locations/+page.svelte @@ -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 { if (inflight >= MAX_INFLIGHT) await new Promise((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(s.locations, []).filter((l) => l != null); spoolOrders = parseSetting>(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(); }; });