From cee060a13751953af95b483f579153497d88ce9d Mon Sep 17 00:00:00 2001 From: "Patrick W. Healy" Date: Wed, 16 Sep 2026 21:50:15 +0000 Subject: [PATCH] frontend: harden selection cancellation and proactive detail expiry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d2243398-6c36-4c3d-969e-7ed7bfb5b459 --- frontend/src/App.tsx | 12 ++++--- frontend/src/hooks/useNodeDetails.ts | 5 ++- frontend/src/state/nodeDetails.ts | 40 ++++++++++++++++------ frontend/tests/nodeDetails.test.ts | 51 ++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 16 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 55617d123..5ed4a0328 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -26,7 +26,7 @@ export default function App() { const [hiddenSites, setHiddenSites] = useState>(new Set()); const [hiddenGatewayPools, setHiddenGatewayPools] = useState>(new Set()); const [selectedNodeName, setSelectedNodeName] = useState(null); - const { detail, load: loadNodeDetail } = useNodeDetails(selectedNodeName); + const { detail, load: loadNodeDetail, cancel: cancelNodeDetail } = useNodeDetails(selectedNodeName); const [selectedNodeDetailTab, setSelectedNodeDetailTab] = useState<'peerings' | 'routes' | 'bpf'>('peerings'); const [pullEnabledOptimistic, setPullEnabledOptimistic] = useState(null); const [selectedNodeTypesFilter, setSelectedNodeTypesFilter] = useState>(new Set(['Gateway', 'Worker'])); @@ -178,12 +178,14 @@ export default function App() { }; const handleSelectNode = useCallback((nodeName: string) => { + cancelNodeDetail(); setSelectedNodeName(nodeName); - }, []); + }, [cancelNodeDetail]); const handleCloseModal = useCallback(() => { + cancelNodeDetail(); setSelectedNodeName(null); - }, []); + }, [cancelNodeDetail]); const { effectivePullEnabled, @@ -216,9 +218,9 @@ export default function App() { // Check if node still exists in the cluster const exists = nodeSummaries.some((ns) => ns.name === selectedNodeName); if (!exists) { - setSelectedNodeName(null); + handleCloseModal(); } - }, [nodeSummaries, selectedNodeName]); + }, [nodeSummaries, selectedNodeName, handleCloseModal]); const wsState = wsConnected ? 'ok' : summary ? 'warn' : 'err'; const wsLabel = wsConnected diff --git a/frontend/src/hooks/useNodeDetails.ts b/frontend/src/hooks/useNodeDetails.ts index 6480df365..4df04dedf 100644 --- a/frontend/src/hooks/useNodeDetails.ts +++ b/frontend/src/hooks/useNodeDetails.ts @@ -23,8 +23,11 @@ export default function useNodeDetails(selectedNodeName: string | null) { const load = useCallback((forceRefresh = false) => { if (selectedNodeName) store.load(selectedNodeName, forceRefresh); }, [store, selectedNodeName]); + const cancel = useCallback(() => { + if (selectedNodeName) store.cancel(selectedNodeName); + }, [store, selectedNodeName]); return { detail: selectedNodeName ? store.read(selectedNodeName) : { state: 'not-loaded' as const }, - load, + load, cancel, }; } diff --git a/frontend/src/state/nodeDetails.ts b/frontend/src/state/nodeDetails.ts index e438e54d6..432412ebe 100644 --- a/frontend/src/state/nodeDetails.ts +++ b/frontend/src/state/nodeDetails.ts @@ -61,6 +61,18 @@ export class NodeDetails { this.expiryTimers.delete(name); } + private scheduleExpiry(name: string) { + this.clearExpiry(name); + const snapshot = this.views.get(name)?.snapshot; + if (!snapshot) return; + const delay = Date.parse(snapshot.expiresAt) - this.clock.now(); + this.expiryTimers.set(name, this.clock.setTimeout(() => { + this.expire(name); + if (this.views.get(name)?.snapshot) this.scheduleExpiry(name); + this.changed(); + }, Math.min(Math.max(0, delay), 2147483647))); + } + private publish(name: string, view: DetailView) { this.views.set(name, view); this.changed(); @@ -89,6 +101,7 @@ export class NodeDetails { load(name: string, forceRefresh = false) { if (!name) return; + if (!forceRefresh && this.operations.has(name)) return; const view = this.read(name); if (!forceRefresh && view.snapshot) { this.publish(name, { state: 'loaded', snapshot: view.snapshot }); @@ -122,6 +135,17 @@ export class NodeDetails { this.publish(name, { state, error, snapshot: this.read(name).snapshot }); } + private armDeadline(name: string, op: Operation) { + op.timer = this.clock.setTimeout(() => { + if (!this.current(name, op)) return; + if (this.clock.now() >= op.deadline) { + this.fail(name, op, 'Detail request deadline expired', 'expired'); + } else { + this.armDeadline(name, op); + } + }, Math.min(Math.max(0, op.deadline - this.clock.now()), 2147483647)); + } + private accept(name: string, op: Operation, result: NodeDetailResult) { if (!this.current(name, op)) return; if (this.clock.now() >= op.deadline) { @@ -146,17 +170,17 @@ export class NodeDetails { this.fail(name, op, 'Detail request deadline expired', 'expired'); return; } - this.publish(name, { ...this.read(name), state: 'loading', deadline: new Date(op.deadline).toISOString() }); + this.publish(name, { + ...this.read(name), state: 'loading', error: result.error, + deadline: new Date(op.deadline).toISOString(), + }); op.timer = this.clock.setTimeout(() => { if (!this.current(name, op)) return; if (this.clock.now() >= op.deadline) { this.fail(name, op, 'Detail request deadline expired', 'expired'); return; } - op.timer = this.clock.setTimeout( - () => this.fail(name, op, 'Detail request deadline expired', 'expired'), - op.deadline - this.clock.now() - ); + this.armDeadline(name, op); void this.transport.poll(name, op.requestId!, op.abort.signal) .then((next) => this.accept(name, op, next)) .catch((error) => this.fail(name, op, String(error.message || error))); @@ -179,11 +203,7 @@ export class NodeDetails { return; } this.finish(name, op); - this.clearExpiry(name); this.publish(name, { state: 'loaded', snapshot }); - this.expiryTimers.set(name, this.clock.setTimeout(() => { - this.expire(name); - this.changed(); - }, expiry - this.clock.now())); + this.scheduleExpiry(name); } } diff --git a/frontend/tests/nodeDetails.test.ts b/frontend/tests/nodeDetails.test.ts index 0214eaa42..5404e3d54 100644 --- a/frontend/tests/nodeDetails.test.ts +++ b/frontend/tests/nodeDetails.test.ts @@ -69,6 +69,7 @@ test('expiry actively releases data and read-time expiry cannot extend TTL', asy f.advance(4999); assert.equal(f.store.read('node').state, 'loaded'); f.advance(5000, runTimers); + if (runTimers) assert.equal(f.store['views'].get('node').snapshot, undefined, 'expiry releases data before a read'); assert.deepEqual(f.store.read('node'), { state: 'expired', error: undefined, deadline: undefined }); assert.equal(f.calls.length, 1); } @@ -141,3 +142,53 @@ test('dispose cancels pending work, clears timers/cache, and ignores late result await tick(); assert.equal(f.store.read('node').snapshot, undefined); }); + +test('loading twice joins a browser waiter; pending GET failures retain valid previous data', async () => { + const f = fixture(); + f.store.load('node'); + f.store.load('node'); + assert.equal(f.calls.length, 1); + f.calls[0].resolve(f.complete()); + await tick(); + f.store.load('node', true); + f.calls[1].resolve({ state: 'pending', nodeName: 'node', requestId: 'new', deadline: time(4000) }); + await tick(); + f.store.load('node'); + assert.equal(f.store.read('node').state, 'loading'); + f.advance(2000); + f.calls[2].reject(new Error('GET failed')); + await tick(); + assert.equal(f.store.read('node').state, 'error'); + assert.equal(f.store.read('node').snapshot.requestId, 'a'); + f.advance(5000); + assert.equal(f.store['views'].get('node').snapshot, undefined); +}); + +test('large TTLs rearm browser-safe timers without expiring early', async () => { + const f = fixture(); + const expires = 2147483647 + 5000; + f.store.load('node'); + f.calls[0].resolve(f.complete('long-lived', expires)); + await tick(); + f.advance(2147483647 + 1000); + assert.equal(f.store.read('node').state, 'loaded'); + f.advance(expires); + assert.equal(f.store['views'].get('node').snapshot, undefined); +}); + +test('initial POST timeout and GET identity mismatch cannot revive snapshots', async () => { + const f = fixture(); + f.store.load('node'); + f.advance(121000); + assert.equal(f.calls[0].signal.aborted, true); + f.calls[0].resolve(f.complete('late', 150000)); + await tick(); + assert.equal(f.store.read('node').snapshot, undefined); + f.store.load('node'); + f.calls[1].resolve({ state: 'pending', nodeName: 'node', requestId: 'expected', deadline: time(150000) }); + await tick(); + f.advance(122000); + f.calls[2].resolve(f.complete('wrong', 150000)); + await tick(); + assert.match(f.store.read('node').error, /Mismatched/); +});