From 6a6401a7d12611c78e052ace816bb09dcfb80cd9 Mon Sep 17 00:00:00 2001 From: Aniruddha Adak Date: Sun, 4 Oct 2026 15:18:41 +0530 Subject: [PATCH] fix(client): clear a poll's own error once the poll recovers The banner is fed by three things: the state poll, the capture poll, and the actions the owner takes. Only the actions ever cleared it. const [s, w] = await Promise.all([api('/state'), api('/workspace')]); setState(s); setWorkspace(w); // no setError('') So a failure the poll reported was latched for the life of the page. Restart the server, or lose the network for five seconds, and the app comes back visibly working - the sidebar, the task list and the chat all repaint from the very request that succeeded - while a red banner still says it is broken. The only way out was the Dismiss button, on an error that was no longer true. The capture poll twenty lines below had the same shape and the same gap, so it is fixed here too rather than leaving one copy of the bug behind. Neither poll clears the banner unconditionally. They remember the message they last failed with and clear only that, because the banner is shared: an unconditional setError('') would take away the message from a failed action the owner had not finished reading, one poll tick after it appeared. That is the same rule SpaceWorkspace's own poll already follows by clearing on success, and it is what afterPollSuccess states on its own so it can be tested without a DOM. --- src/client/App.tsx | 29 +++++++++++++++++++++++------ src/client/poll-error.ts | 11 +++++++++++ tests/poll-error.test.ts | 31 +++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 src/client/poll-error.ts create mode 100644 tests/poll-error.test.ts diff --git a/src/client/App.tsx b/src/client/App.tsx index 353b631b..29954434 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -1,4 +1,5 @@ import { openPageLink } from './page-navigation'; +import { afterPollSuccess } from './poll-error'; import { SpaceNav } from './SpaceNav'; import { SpaceWorkspace } from './SpaceWorkspace'; import { useCallback, useEffect, useState, useRef } from 'react'; @@ -105,6 +106,8 @@ export function App() { const [busy, setBusy] = useState(false); const [search, setSearch] = useState(''); const [taskDetail, setTaskDetail] = useState(); + const pollError = useRef(undefined); + const captureError = useRef(undefined); const refresh = useCallback(async () => { try { const [s, w] = await Promise.all([ @@ -115,12 +118,18 @@ export function App() { setWorkspace(w); setNeedsAuth(false); setSelectedDot((previous) => previous || w.dots[0]?.id || ''); + // The server answered, so the failure this poll was reporting is over. Anything else on + // screen belongs to something the owner did and stays until they dismiss it. + setError((current) => afterPollSuccess(current, pollError.current)); + pollError.current = undefined; } catch (e) { if (e instanceof ApiError && e.status === 401) setNeedsAuth(true); - else - setError( - e instanceof Error ? e.message : 'Could not connect to the server.', - ); + else { + const message = + e instanceof Error ? e.message : 'Could not connect to the server.'; + pollError.current = message; + setError(message); + } } }, []); useEffect(() => { @@ -132,13 +141,21 @@ export function App() { setCapture(undefined); if (!selectedThread) return; let active = true; + captureError.current = undefined; const load = () => void api(`/conversations/${selectedThread}/capture`) .then((result) => { - if (active) setCapture(result ?? undefined); + if (!active) return; + setCapture(result ?? undefined); + setError((current) => + afterPollSuccess(current, captureError.current), + ); + captureError.current = undefined; }) .catch((e) => { - if (active) setError(e.message); + if (!active) return; + captureError.current = e.message; + setError(e.message); }); load(); const timer = setInterval(load, 3000); diff --git a/src/client/poll-error.ts b/src/client/poll-error.ts new file mode 100644 index 00000000..44c727e1 --- /dev/null +++ b/src/client/poll-error.ts @@ -0,0 +1,11 @@ +/** + * A poll reports what went wrong, and clears it again once the thing it was watching answers. + * It must not clear anything else: the banner is shared with the errors from actions the owner + * just took, and a poll that succeeds a moment later would otherwise take those away before + * they had been read. + * + * `raised` is the message this poll last failed with, and `current` whatever is on screen now. + */ +export function afterPollSuccess(current: string, raised?: string): string { + return raised !== undefined && current === raised ? '' : current; +} diff --git a/tests/poll-error.test.ts b/tests/poll-error.test.ts new file mode 100644 index 00000000..bddc41a8 --- /dev/null +++ b/tests/poll-error.test.ts @@ -0,0 +1,31 @@ +import { expect, it } from 'vitest'; +import { afterPollSuccess } from '../src/client/poll-error'; + +it('clears the failure the poll itself raised', () => { + expect(afterPollSuccess('Failed to fetch', 'Failed to fetch')).toBe(''); +}); + +it('leaves an error the owner is looking at from something they just did', () => { + // A failed action reported its own error; a poll that happens to succeed a moment later must + // not take it away before it has been read. + expect( + afterPollSuccess('That schedule interval is too short.', 'Failed to fetch'), + ).toBe('That schedule interval is too short.'); +}); + +it('leaves an error alone when the poll has not failed yet', () => { + expect( + afterPollSuccess('That folder is no longer available.', undefined), + ).toBe('That folder is no longer available.'); +}); + +it('stays empty when the owner already dismissed the banner', () => { + expect(afterPollSuccess('', 'Failed to fetch')).toBe(''); +}); + +it('does not clear a different message that happens to be showing', () => { + // Two polls can fail for different reasons. Only the one the successful poll raised is its own. + expect( + afterPollSuccess('Could not reach the server.', 'Failed to fetch'), + ).toBe('Could not reach the server.'); +});