From f21abe7af5ef643efc8bb5cffbb019bb22e8f55a Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Tue, 18 Aug 2026 20:45:59 -0400 Subject: [PATCH] feat(ui): one visibility-aware polling convention, and a ratchet on the rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan 76 milestone E2, drift/bug row 4. 45 hand-rolled setInterval sites across 35 files, with `visibilitychange` handled in exactly one of them and an in-flight guard in exactly one of them — the documented poller-stampede shape. Two failures every hand-rolled `setInterval(load, 5000)` shares: - it starts a request every 5s whether or not the previous one returned. A slow endpoint does not slow the poller down, it makes it stack; each extra request makes the server slower, which makes the next overlap more likely. This is what a DevTools capture of /domains showed as a wall of pending rows. - it keeps polling a tab nobody is looking at, where browsers throttle timers unevenly anyway, so it is not even a reliable refresh. utils/pollScheduler.js holds the rules as a plain object so they are testable without a DOM (the repo has no jsdom — this follows formState.js/useForm). hooks/usePolling.js is the React binding, and useServerQuery gains `refetchInterval` built on the same scheduler, so there is one implementation rather than a second one for query users. Two design points worth stating, since both differ from what the sites did: - the next tick is armed by the previous run *finishing*, not by setInterval. With setInterval the gap is measured tick-to-tick regardless of how long the work took, so slow work silently turns a 5s gap into no gap. The in-flight guard therefore protects the out-of-band paths — refresh() and the visibility catch-up — rather than the schedule, and the test says so instead of implying a guard the schedule can no longer exercise. - the callback is held in a ref, so an inline arrow re-created each render does not restart the timer. That is the usual reason a hand-rolled poller runs far faster than its stated interval. Migrated four as reference implementations, per the plan's "migrate a representative flow before broader adoption": DeployPill (which had worked all of this out by hand and loses ~20 lines), MonitorsSummary, appdetail/LogsTab, and ProcessListPanel. MonitorsSummary keeps its `refreshKey` prop as an explicit out-of-band refresh rather than folding it into the schedule. Ratchet: check-frontend-boundaries.mjs gains LEGACY_POLLERS with exact per-file counts, now 41 sites in 31 files, so every future cleanup must shrink it and a new bare setInterval fails lint. Genuine non-fetching timers (clock ticks, countdowns) are listed too rather than pattern-matched away — they are harmless but the exact count is what makes the ratchet trustworthy. Gate: npm test (72 passed, 11 of them new), npm run lint (0 errors; 954 warnings, one fewer than the 955 baseline), npm run build clean, boundary check green. Co-Authored-By: Claude Opus 5 (1M context) --- .../scripts/check-frontend-boundaries.mjs | 67 +++- frontend/src/components/DeployPill.jsx | 22 +- frontend/src/components/appdetail/LogsTab.jsx | 11 +- .../components/databases/ProcessListPanel.jsx | 10 +- .../components/monitoring/MonitorsSummary.jsx | 14 +- frontend/src/hooks/usePolling.js | 84 +++++ frontend/src/hooks/useServerQuery.js | 13 + .../utils/__tests__/pollScheduler.test.mjs | 313 ++++++++++++++++++ frontend/src/utils/pollScheduler.js | 127 +++++++ 9 files changed, 628 insertions(+), 33 deletions(-) create mode 100644 frontend/src/hooks/usePolling.js create mode 100644 frontend/src/utils/__tests__/pollScheduler.test.mjs create mode 100644 frontend/src/utils/pollScheduler.js diff --git a/frontend/scripts/check-frontend-boundaries.mjs b/frontend/scripts/check-frontend-boundaries.mjs index 2109e078..d2b9c9fb 100644 --- a/frontend/scripts/check-frontend-boundaries.mjs +++ b/frontend/scripts/check-frontend-boundaries.mjs @@ -34,6 +34,55 @@ const EXTENSION_OWNED_STYLES = [ // insecure context, and SSL is optional by policy. This map must stay empty. const LEGACY_CLIPBOARD = new Map(); +// Polling belongs to usePolling / useServerQuery({refetchInterval}), which do +// two things a hand-rolled setInterval does not: they never start a request +// while the previous one is still in flight, and they stop entirely while the +// tab is hidden, catching up once on return. A bare setInterval that fetches is +// the documented poller-stampede shape - the page gets slower the longer it is +// left open. Finite legacy baseline with exact counts, so every cleanup shrinks +// it. A genuine non-fetching timer (a clock tick, a countdown) may stay, but it +// is still listed so nothing new slips in unnoticed. +const LEGACY_POLLERS = new Map(Object.entries({ + 'components/dashboard/widgets/renderers.jsx': 1, + 'components/deploy-console/SuccessBanner.jsx': 1, + 'components/docker/ContainersTab.jsx': 2, + 'components/monitoring/DoctorPanel.jsx': 1, + 'components/security/ScannerTab.jsx': 2, + 'components/security/VulnerabilityTab.jsx': 1, + 'components/server/OnboardingWizard.jsx': 2, + 'components/serverdetail/ServicesTab.jsx': 1, + 'components/servers/LinkPanelForm.jsx': 1, + 'components/service-detail/LogsTab.jsx': 1, + 'components/service-detail/MetricsTab.jsx': 1, + 'components/service-detail/OverviewTab.jsx': 1, + 'hooks/useContainerStatus.js': 1, + 'hooks/useDeployJobStream.js': 1, + 'hooks/useMetrics.js': 1, + 'pages/Dashboard.jsx': 4, + 'pages/Databases.jsx': 1, + 'pages/DeliveryLog.jsx': 1, + 'pages/DeployConsole.jsx': 1, + 'pages/Deployments.jsx': 1, + 'pages/ImportWizard.jsx': 1, + 'pages/Jobs.jsx': 1, + 'pages/MonitorDetail.jsx': 1, + 'pages/Monitors.jsx': 2, + 'pages/QueueDetail.jsx': 1, + 'pages/QueueOperations.jsx': 1, + 'pages/ServerDetail.jsx': 1, + 'pages/Terminal.jsx': 3, + 'pages/TestSandbox.jsx': 2, + 'plugins/serverkit-gui/components/ServerGui.jsx': 1, + 'plugins/serverkit-gui/components/SyntheticDesktop.jsx': 1, +})); + +// The polling door itself, plus the query hook that wraps it. +const POLLING_DOOR = new Set([ + 'utils/pollScheduler.js', + 'hooks/usePolling.js', + 'hooks/useServerQuery.js', +]); + // These are purpose-built experiences whose geometry/content is intentionally // richer than the ordinary Modal contract. Any new exception needs review. const LOW_LEVEL_DIALOG_EXCEPTIONS = new Set([ @@ -52,6 +101,7 @@ function walk(dir) { const files = walk(src).filter((path) => ['.js', '.jsx'].includes(extname(path))); const failures = []; const seenClipboard = new Set(); +const seenPollers = new Set(); function rel(path) { return relative(src, path).replaceAll('\\', '/'); @@ -74,6 +124,15 @@ for (const path of files) { if (expected) seenClipboard.add(file); } + if (!POLLING_DOOR.has(file)) { + const actual = count(source, /setInterval\s*\(/g); + const expected = LEGACY_POLLERS.get(file) || 0; + if (actual !== expected) { + failures.push(`${file}: raw setInterval count is ${actual}; legacy baseline is ${expected}. Use usePolling() (or useServerQuery({refetchInterval})) and shrink the baseline.`); + } + if (expected) seenPollers.add(file); + } + if (/(?:(?:window|globalThis)\s*\.\s*)?confirm\s*\(\s*['"`]/m.test(source)) { failures.push(`${file}: use useConfirm() instead of the browser confirm dialog.`); } @@ -117,6 +176,12 @@ for (const file of LEGACY_CLIPBOARD.keys()) { } } +for (const file of LEGACY_POLLERS.keys()) { + if (!seenPollers.has(file)) { + failures.push(`${file}: legacy poller entry is stale; remove it from the boundary baseline.`); + } +} + const mainStylesPath = resolve(src, 'styles', 'main.scss'); const mainStyles = readFileSync(mainStylesPath, 'utf8'); for (const ownership of EXTENSION_OWNED_STYLES) { @@ -159,4 +224,4 @@ if (failures.length) { process.exit(1); } -console.log(`✓ frontend boundaries: browser/API/dialog boundaries and ${EXTENSION_OWNED_STYLES.length} extension style ownership rule(s) hold (${LEGACY_CLIPBOARD.size} clipboard files remain ratcheted).`); +console.log(`✓ frontend boundaries: browser/API/dialog boundaries and ${EXTENSION_OWNED_STYLES.length} extension style ownership rule(s) hold (${LEGACY_CLIPBOARD.size} clipboard / ${LEGACY_POLLERS.size} poller files remain ratcheted).`); diff --git a/frontend/src/components/DeployPill.jsx b/frontend/src/components/DeployPill.jsx index 44fc5486..7541c053 100644 --- a/frontend/src/components/DeployPill.jsx +++ b/frontend/src/components/DeployPill.jsx @@ -3,6 +3,7 @@ import { Link, useLocation } from 'react-router-dom'; import { Loader2 } from 'lucide-react'; import api from '../services/api'; import socketService from '../services/socket'; +import { usePolling } from '../hooks/usePolling'; // A small global "deploy in progress" pill: while any deployment job is // pending/running it appears (bottom-left) so you can navigate away mid-deploy @@ -49,30 +50,19 @@ export default function DeployPill() { } }, []); + // The interval, the hidden-tab pause, and the catch-up on return are the + // shared convention now; this component was where they were first worked + // out by hand. + usePolling(refresh, POLL_MS, { enabled: !onDeploySurface }); + useEffect(() => { if (onDeploySurface) return undefined; - - // Polling a background tab buys nothing and competes for the browser's - // per-origin connections with whatever the visible tab is doing. - const tick = () => { - if (document.visibilityState !== 'visible') return; - refresh(); - }; - tick(); - const t = setInterval(tick, POLL_MS); - - // Catch up once when the tab comes back rather than polling all along. - const onVisible = () => { if (document.visibilityState === 'visible') refresh(); }; - document.addEventListener('visibilitychange', onVisible); - // React quickly to live status changes when a console elsewhere is // streaming — but a burst of deploy_status events must not become a // burst of requests, so the in-flight guard covers these too. const unsubStatus = socketService.on('deploy_status', refresh); const unsubConnect = socketService.on('connected', refresh); return () => { - clearInterval(t); - document.removeEventListener('visibilitychange', onVisible); unsubStatus(); unsubConnect(); }; diff --git a/frontend/src/components/appdetail/LogsTab.jsx b/frontend/src/components/appdetail/LogsTab.jsx index 7f3c7069..fe991f56 100644 --- a/frontend/src/components/appdetail/LogsTab.jsx +++ b/frontend/src/components/appdetail/LogsTab.jsx @@ -2,6 +2,9 @@ import { useState, useEffect } from 'react'; import { logsToText } from '@/utils/logText'; import api from '../../services/api'; import { Button } from '@/components/ui/button'; +import { usePolling } from '../../hooks/usePolling'; + +const LOG_REFRESH_MS = 5000; const LogsTab = ({ app }) => { const [logs, setLogs] = useState(''); @@ -15,11 +18,9 @@ const LogsTab = ({ app }) => { loadLogs(); }, [app.id]); - useEffect(() => { - if (!autoRefresh) return; - const interval = setInterval(loadLogs, 5000); - return () => clearInterval(interval); - }, [autoRefresh, app.id]); + // immediate: false — the effect above already loads on mount and on app + // change; this adds only the repeat while auto-refresh is on. + usePolling(loadLogs, LOG_REFRESH_MS, { enabled: autoRefresh, immediate: false }); async function loadLogs() { try { diff --git a/frontend/src/components/databases/ProcessListPanel.jsx b/frontend/src/components/databases/ProcessListPanel.jsx index 3648d360..c6f9644b 100644 --- a/frontend/src/components/databases/ProcessListPanel.jsx +++ b/frontend/src/components/databases/ProcessListPanel.jsx @@ -3,6 +3,7 @@ import { Ban, Activity, TimerReset } from 'lucide-react'; import api from '../../services/api'; import { useToast } from '../../contexts/ToastContext'; import { useConfirm } from '../../hooks/useConfirm'; +import { usePolling } from '../../hooks/usePolling'; import EmptyState from '../EmptyState'; import { DataTable, DataTableFooter, SearchField } from '@/components/ds'; import { @@ -114,11 +115,10 @@ export default function ProcessListPanel({ conn, engine, active, isAdmin }) { useEffect(() => { load(); }, [load]); - useEffect(() => { - if (!active || !autoRefresh) return undefined; - const t = setInterval(() => load(true), REFRESH_MS); - return () => clearInterval(t); - }, [active, autoRefresh, load]); + usePolling(() => load(true), REFRESH_MS, { + enabled: active && autoRefresh, + immediate: false, // the effect above already loaded once + }); async function killProcess(proc) { const verb = engineKind === 'postgresql' ? 'Terminate' : 'Kill'; diff --git a/frontend/src/components/monitoring/MonitorsSummary.jsx b/frontend/src/components/monitoring/MonitorsSummary.jsx index b15d3550..a65cb5be 100644 --- a/frontend/src/components/monitoring/MonitorsSummary.jsx +++ b/frontend/src/components/monitoring/MonitorsSummary.jsx @@ -5,13 +5,14 @@ // machines ServerKit runs on, and nothing described the things it is supposed to // be watching. Host health stays below; this sits above it because "is the site // up" is the question the page is actually for. -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { Link, useNavigate } from 'react-router-dom'; import { Activity, Plus, Radar, ShieldAlert, Timer, Zap } from 'lucide-react'; import api from '../../services/api'; import { AreaChart, KpiBand, MetricCard, Pill } from '@/components/ds'; import { Button } from '@/components/ui/button'; import { monitorStateOf } from './monitorShared'; +import { usePolling } from '../../hooks/usePolling'; const POLL_MS = 15000; const FEED_LIMIT = 9; @@ -32,7 +33,6 @@ export default function MonitorsSummary({ refreshKey = 0 }) { const [stats, setStats] = useState(null); const [feed, setFeed] = useState([]); const [loaded, setLoaded] = useState(false); - const pollRef = useRef(null); const load = useCallback(async () => { try { @@ -60,11 +60,13 @@ export default function MonitorsSummary({ refreshKey = 0 }) { } }, []); + const refresh = usePolling(load, POLL_MS); + + // The parent bumps refreshKey to force a reload (e.g. after creating a + // monitor); it is not part of the polling schedule. useEffect(() => { - load(); - pollRef.current = setInterval(load, POLL_MS); - return () => clearInterval(pollRef.current); - }, [load, refreshKey]); + if (refreshKey) refresh(); + }, [refreshKey, refresh]); if (!loaded) return null; diff --git a/frontend/src/hooks/usePolling.js b/frontend/src/hooks/usePolling.js new file mode 100644 index 00000000..dd9130e3 --- /dev/null +++ b/frontend/src/hooks/usePolling.js @@ -0,0 +1,84 @@ +import { useCallback, useEffect, useRef } from 'react'; + +import { createPollScheduler, DEFAULT_INTERVAL_MS } from '@/utils/pollScheduler'; + +// One convention for "keep this fresh while the user is looking at it". +// +// usePolling(loadStatus, 5000); +// usePolling(loadLogs, 3000, { enabled: autoRefresh }); +// +// Replaces the `useEffect(() => { const t = setInterval(load, 5000); return +// () => clearInterval(t); })` block, which this codebase had ~47 of and which +// gets two things wrong every time: +// +// * it starts a new request every 5s whether or not the last one came back, +// so a slow endpoint turns into a growing pile of concurrent requests; +// * it keeps polling a tab nobody is looking at. +// +// The scheduling itself lives in utils/pollScheduler.js so it can be tested +// without a DOM; this is the React binding. +// +// `callback` is held in a ref, so an inline arrow re-created every render does +// NOT restart the timer — the usual reason a hand-rolled poller silently polls +// far faster than its stated interval. +// +// Returns a stable `refresh()` for "reload now" buttons, which goes through the +// same in-flight guard. +export function usePolling(callback, intervalMs = DEFAULT_INTERVAL_MS, options = {}) { + const { + enabled = true, + immediate = true, + pauseWhenHidden = true, + onError, + } = options; + + const callbackRef = useRef(callback); + const onErrorRef = useRef(onError); + callbackRef.current = callback; + onErrorRef.current = onError; + + const schedulerRef = useRef(null); + + useEffect(() => { + if (!enabled || !intervalMs) return undefined; + + const scheduler = createPollScheduler({ + run: () => callbackRef.current?.(), + intervalMs, + setTimer: (fn, ms) => window.setTimeout(fn, ms), + clearTimer: (handle) => window.clearTimeout(handle), + isHidden: () => ( + pauseWhenHidden + && typeof document !== 'undefined' + && document.visibilityState === 'hidden' + ), + onError: (error) => onErrorRef.current?.(error), + }); + schedulerRef.current = scheduler; + scheduler.start({ runNow: immediate }); + + let onVisibility; + if (pauseWhenHidden && typeof document !== 'undefined') { + onVisibility = () => scheduler.setHidden(document.visibilityState === 'hidden'); + document.addEventListener('visibilitychange', onVisibility); + } + + return () => { + if (onVisibility) document.removeEventListener('visibilitychange', onVisibility); + scheduler.stop(); + schedulerRef.current = null; + }; + }, [enabled, intervalMs, immediate, pauseWhenHidden]); + + return useCallback(() => { + const scheduler = schedulerRef.current; + if (scheduler) { + scheduler.refresh(); + return; + } + // Disabled or unmounted: a manual refresh should still do the work. + callbackRef.current?.(); + }, []); +} + +export default usePolling; diff --git a/frontend/src/hooks/useServerQuery.js b/frontend/src/hooks/useServerQuery.js index 98960e92..ef6c660f 100644 --- a/frontend/src/hooks/useServerQuery.js +++ b/frontend/src/hooks/useServerQuery.js @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react'; import { useWorkspace } from '../contexts/WorkspaceContext'; import { queryClient, serializeQueryKey } from '../services/queryClient'; +import { usePolling } from './usePolling'; const workspaceKey = (workspaceId, queryKey) => ['workspace', workspaceId, ...queryKey]; @@ -8,6 +9,11 @@ export function useServerQuery(queryKey, queryFn, options = {}) { const { enabled = true, staleTime = 0, + // Keep this key fresh on an interval. Two things a hand-rolled + // setInterval next to a useState does not get: the query client already + // dedupes concurrent fetches of the same key, and the scheduler below + // stops entirely while the tab is hidden, catching up once on return. + refetchInterval = 0, onError, } = options; const { activeWorkspaceId } = useWorkspace(); @@ -60,6 +66,13 @@ export function useServerQuery(queryKey, queryFn, options = {}) { [scopedKeyHash, staleTime], ); + usePolling(refetch, refetchInterval, { + enabled: enabled && refetchInterval > 0, + // The mount effect above already fetched; polling adds the *repeat*. + immediate: false, + onError: (error) => onErrorRef.current?.(error), + }); + return { ...state, isLoading: state.status === 'idle' || state.status === 'loading', diff --git a/frontend/src/utils/__tests__/pollScheduler.test.mjs b/frontend/src/utils/__tests__/pollScheduler.test.mjs new file mode 100644 index 00000000..396d6027 --- /dev/null +++ b/frontend/src/utils/__tests__/pollScheduler.test.mjs @@ -0,0 +1,313 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { createPollScheduler } from '../pollScheduler.js'; + +// A hand-driven clock. The scheduler only ever talks to timers through the +// injected pair, so a test can advance time deterministically. +function fakeClock() { + let next = 1; + const pending = new Map(); + return { + setTimer(fn, ms) { + const id = next++; + pending.set(id, { fn, ms }); + return id; + }, + clearTimer(id) { pending.delete(id); }, + /** Fire every timer currently scheduled (one round). */ + async fire() { + const round = [...pending.entries()]; + round.forEach(([id]) => pending.delete(id)); + round.forEach(([, t]) => t.fn()); + await Promise.resolve(); + await Promise.resolve(); + }, + get scheduled() { return pending.size; }, + }; +} + +function deferred() { + let resolve; + let reject; + const promise = new Promise((res, rej) => { resolve = res; reject = rej; }); + return { promise, resolve, reject }; +} + + +test('runs immediately on start, then on each tick', async () => { + const clock = fakeClock(); + let runs = 0; + const poller = createPollScheduler({ + run: () => { runs += 1; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + assert.equal(runs, 1, 'start() should run once immediately'); + + await clock.fire(); + assert.equal(runs, 2); + await clock.fire(); + assert.equal(runs, 3); + + poller.stop(); +}); + + +test('start({runNow:false}) waits for the first interval', async () => { + const clock = fakeClock(); + let runs = 0; + const poller = createPollScheduler({ + run: () => { runs += 1; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start({ runNow: false }); + assert.equal(runs, 0); + await clock.fire(); + assert.equal(runs, 1); + poller.stop(); +}); + + +test('the schedule itself can never overlap a request', async () => { + // The stampede this module exists to prevent. Note the schedule cannot + // produce one *structurally*: the next timer is armed by the previous run + // finishing, so while a request is in flight there is no timer at all. + // That is the property worth pinning — a guard you can also delete without + // any test noticing would be no guard. + const clock = fakeClock(); + const first = deferred(); + let starts = 0; + const poller = createPollScheduler({ + run: () => { starts += 1; return first.promise; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + assert.equal(starts, 1); + assert.equal(clock.scheduled, 0, 'no timer may be armed while a request is in flight'); + + await clock.fire(); + await clock.fire(); + assert.equal(starts, 1, 'nothing to fire, so nothing stacked'); + + first.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + await clock.fire(); + assert.equal(starts, 2, 'polling resumes once the request settles'); + poller.stop(); +}); + + +test('out-of-band runs are what the in-flight guard actually protects', async () => { + // refresh() and the visibility catch-up bypass the schedule, so they are + // the paths that CAN land on top of a request in flight. + const clock = fakeClock(); + const pending = deferred(); + let starts = 0; + const poller = createPollScheduler({ + run: () => { starts += 1; return pending.promise; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + isHidden: () => false, + }); + + poller.start(); + assert.equal(starts, 1); + + poller.refresh(); + poller.refresh(); + poller.setHidden(false); // a catch-up while the first is still open + poller.start(); // and a redundant start + + assert.equal(starts, 1, 'none of these may stack a second request'); + assert.equal(poller.skippedTicks, 4); + + pending.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await clock.fire(); + assert.equal(starts, 2); + poller.stop(); +}); + + +test('a skipped tick does not queue up a burst afterwards', async () => { + // Skipping must mean "drop it", not "remember it". Otherwise the stampede + // just arrives later, all at once. + const clock = fakeClock(); + const slow = deferred(); + let starts = 0; + const poller = createPollScheduler({ + run: () => { starts += 1; return starts === 1 ? slow.promise : Promise.resolve(); }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + for (let i = 0; i < 5; i += 1) await clock.fire(); + assert.equal(starts, 1); + + slow.resolve(); + await Promise.resolve(); + await Promise.resolve(); + + await clock.fire(); + assert.equal(starts, 2, 'exactly one run, not five'); + assert.equal(clock.scheduled, 1, 'and exactly one timer outstanding'); + poller.stop(); +}); + + +test('the gap is measured after the work finishes, not tick to tick', async () => { + const clock = fakeClock(); + let starts = 0; + let pendingRun = deferred(); + const poller = createPollScheduler({ + run: () => { starts += 1; return pendingRun.promise; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + assert.equal(clock.scheduled, 0, 'nothing is scheduled while work is in flight'); + + pendingRun.resolve(); + await Promise.resolve(); + await Promise.resolve(); + assert.equal(clock.scheduled, 1, 'the next tick is scheduled only once work settled'); + + pendingRun = deferred(); + await clock.fire(); + assert.equal(starts, 2); + poller.stop(); +}); + + +test('stops polling while the tab is hidden and catches up when it returns', async () => { + const clock = fakeClock(); + let hidden = false; + let runs = 0; + const poller = createPollScheduler({ + run: () => { runs += 1; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + isHidden: () => hidden, + }); + + poller.start(); + assert.equal(runs, 1); + + hidden = true; + poller.setHidden(true); + assert.equal(clock.scheduled, 0, 'no timer should be armed for a hidden tab'); + await clock.fire(); + assert.equal(runs, 1, 'a hidden tab must not poll'); + + hidden = false; + poller.setHidden(false); + assert.equal(runs, 2, 'coming back runs once immediately to catch up'); + + await clock.fire(); + assert.equal(runs, 3, 'and resumes the schedule'); + poller.stop(); +}); + + +test('stop() cancels the outstanding timer', async () => { + const clock = fakeClock(); + let runs = 0; + const poller = createPollScheduler({ + run: () => { runs += 1; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + poller.stop(); + assert.equal(clock.scheduled, 0); + await clock.fire(); + assert.equal(runs, 1, 'no further runs after stop()'); + assert.equal(poller.isRunning, false); +}); + + +test('a rejected run does not wedge the poller', async () => { + // If a failed request left inFlight true, the poller would go silent + // forever — the worst failure mode, because the page looks live. + const clock = fakeClock(); + let starts = 0; + const errors = []; + const poller = createPollScheduler({ + run: () => { starts += 1; return Promise.reject(new Error('boom')); }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + onError: (error) => errors.push(error.message), + }); + + poller.start(); + await Promise.resolve(); + await Promise.resolve(); + + assert.deepEqual(errors, ['boom'], 'the failure is surfaced, not swallowed'); + + await clock.fire(); + assert.equal(starts, 2, 'polling continues after a failure'); + assert.equal(poller.isInFlight, false); + poller.stop(); +}); + + +test('a synchronously throwing run does not wedge the poller either', async () => { + const clock = fakeClock(); + let starts = 0; + const poller = createPollScheduler({ + run: () => { starts += 1; throw new Error('sync boom'); }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + onError: () => {}, + }); + + poller.start(); + assert.equal(poller.isInFlight, false); + await clock.fire(); + assert.equal(starts, 2); + poller.stop(); +}); + + +test('refresh() honours the in-flight guard', async () => { + const clock = fakeClock(); + const pending = deferred(); + let starts = 0; + const poller = createPollScheduler({ + run: () => { starts += 1; return pending.promise; }, + intervalMs: 1000, + setTimer: clock.setTimer, + clearTimer: clock.clearTimer, + }); + + poller.start(); + poller.refresh(); + poller.refresh(); + assert.equal(starts, 1, 'a manual refresh must not stack either'); + pending.resolve(); + poller.stop(); +}); diff --git a/frontend/src/utils/pollScheduler.js b/frontend/src/utils/pollScheduler.js new file mode 100644 index 00000000..8083a55b --- /dev/null +++ b/frontend/src/utils/pollScheduler.js @@ -0,0 +1,127 @@ +// The scheduling rules behind usePolling, as a plain object so they can be +// tested without a DOM. +// +// Two rules, both of which every hand-rolled `setInterval(load, 5000)` in this +// codebase got wrong: +// +// 1. NEVER let a tick start while the previous one is still in flight. A poller +// on a 5s interval against a request that has started taking 30s does not +// slow down — it stacks, and each new request makes the server slower, which +// makes the next request more likely to overlap. That is the stampede: the +// page gets slower the longer you leave it open, and the server sees a load +// it never sees in a fresh session. +// +// 2. Do not poll a tab nobody is looking at. A background tab polling every 5s +// is pure cost, and browsers throttle its timers unevenly anyway, so it is +// not even a reliable refresh. Catch up ONCE when it comes back instead — +// which is also what makes the data fresh at the moment someone looks. +// +// A skipped tick is not a dropped update: the next one takes its place, and the +// in-flight request that caused the skip is delivering the same data. + +export const DEFAULT_INTERVAL_MS = 5000; + +/** + * @param {object} config + * @param {() => (Promise|any)} config.run the work to repeat + * @param {number} config.intervalMs delay BETWEEN runs + * @param {(fn: Function, ms: number) => any} config.setTimer + * @param {(handle: any) => void} config.clearTimer + * @param {() => boolean} [config.isHidden] document.hidden, injected + * @param {(error: any) => void} [config.onError] + */ +export function createPollScheduler({ + run, + intervalMs = DEFAULT_INTERVAL_MS, + setTimer, + clearTimer, + isHidden = () => false, + onError, +}) { + let handle = null; + let inFlight = false; + let stopped = true; + let skipped = 0; + + const clear = () => { + if (handle !== null) { + clearTimer(handle); + handle = null; + } + }; + + // A timeout chained after each run, not setInterval: with setInterval the + // delay is measured from tick to tick regardless of how long the work took, + // so slow work silently turns a 5s gap into no gap at all. + const schedule = () => { + clear(); + if (stopped || isHidden()) return; + handle = setTimer(tick, intervalMs); + }; + + function tick() { + handle = null; + if (stopped) return; + if (inFlight) { + // Rule 1. Do not queue behind it; the next tick will do. + skipped += 1; + schedule(); + return; + } + if (isHidden()) return; // rule 2; onVisible() resumes + + let result; + inFlight = true; + try { + result = run(); + } catch (error) { + inFlight = false; + onError?.(error); + schedule(); + return; + } + + if (!result || typeof result.then !== 'function') { + inFlight = false; + schedule(); + return; + } + + result.then( + () => { inFlight = false; schedule(); }, + (error) => { inFlight = false; onError?.(error); schedule(); }, + ); + } + + return { + /** Begin polling. `runNow` performs the first run immediately. */ + start({ runNow = true } = {}) { + stopped = false; + if (runNow) tick(); + else schedule(); + }, + stop() { + stopped = true; + clear(); + }, + /** Visibility changed. Coming back runs once immediately to catch up. */ + setHidden(hidden) { + if (stopped) return; + if (hidden) clear(); + else tick(); + }, + /** Run now, outside the schedule, honouring the in-flight guard. */ + refresh() { + if (stopped) { + stopped = false; + } + tick(); + }, + /** Test/diagnostic surface. */ + get isRunning() { return !stopped; }, + get isInFlight() { return inFlight; }, + get skippedTicks() { return skipped; }, + }; +} + +export default createPollScheduler;