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.'); +});