From 3b9443d457b32b27a917a963b000c8aa4d0234af Mon Sep 17 00:00:00 2001 From: Charan Rathore <180254320+charan-rathore@users.noreply.github.com> Date: Sun, 4 Oct 2026 13:44:06 +0530 Subject: [PATCH 1/2] fix: clear the connection error once polling recovers A successful workspace or capture poll now clears only the connection notice, so a recovered server no longer leaves the error banner up. Errors from user actions are kept apart and still need a dismiss. Fixes #51 --- src/client/App.tsx | 50 ++++++++++++++++++++---- src/client/poll-notice.ts | 55 +++++++++++++++++++++++++++ tests/poll-notice.test.ts | 80 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 src/client/poll-notice.ts create mode 100644 tests/poll-notice.test.ts diff --git a/src/client/App.tsx b/src/client/App.tsx index 353b631..456047b 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -32,6 +32,13 @@ import type { WorkspaceState, } from '../shared/types'; import { api, ApiError, authHeaders, setToken } from './api'; +import { + applyCaptureResult, + applyRefreshResult, + dismissNotice, + visibleNotice, + type Notices, +} from './poll-notice'; import { Mascot } from './Mascot'; import { Chat } from './Chat'; import { ThreadList } from './ThreadList'; @@ -40,6 +47,14 @@ import { TaskRow } from './TaskPresentation'; import { TaskActions } from './TaskActions'; import { WorkspaceDialog, type Dialog } from './WorkspaceDialog'; +function describeFailure(error: unknown, fallback: string) { + return { + ok: false as const, + status: error instanceof ApiError ? error.status : undefined, + message: error instanceof Error ? error.message : fallback, + }; +} + export function App() { const [state, setState] = useState(); const [workspace, setWorkspace] = useState(); @@ -92,7 +107,15 @@ export function App() { openPageLink(`#/spaces/${space}${page ? `/pages/${page}` : ''}`); }; - const [error, setError] = useState(''); + const [notices, setNotices] = useState({ + connection: '', + action: '', + }); + const error = visibleNotice(notices); + const setError = (action: string) => + setNotices((current) => + current.action === action ? current : { ...current, action }, + ); const [auth, setAuth] = useState(''); const [needsAuth, setNeedsAuth] = useState(false); const [dialog, setDialog] = useState(); @@ -115,12 +138,15 @@ export function App() { setWorkspace(w); setNeedsAuth(false); setSelectedDot((previous) => previous || w.dots[0]?.id || ''); + setNotices((current) => applyRefreshResult(current, { ok: true })); } catch (e) { if (e instanceof ApiError && e.status === 401) setNeedsAuth(true); - else - setError( - e instanceof Error ? e.message : 'Could not connect to the server.', - ); + setNotices((current) => + applyRefreshResult( + current, + describeFailure(e, 'Could not connect to the server.'), + ), + ); } }, []); useEffect(() => { @@ -135,10 +161,18 @@ export function App() { const load = () => void api(`/conversations/${selectedThread}/capture`) .then((result) => { - if (active) setCapture(result ?? undefined); + if (!active) return; + setCapture(result ?? undefined); + setNotices((current) => applyCaptureResult(current, { ok: true })); }) .catch((e) => { - if (active) setError(e.message); + if (!active) return; + setNotices((current) => + applyCaptureResult( + current, + describeFailure(e, 'Could not connect to the server.'), + ), + ); }); load(); const timer = setInterval(load, 3000); @@ -528,7 +562,7 @@ export function App() { diff --git a/src/client/poll-notice.ts b/src/client/poll-notice.ts new file mode 100644 index 0000000..b049c07 --- /dev/null +++ b/src/client/poll-notice.ts @@ -0,0 +1,55 @@ +export interface Notices { + connection: string; + action: string; +} + +export interface PollFailure { + ok: false; + status?: number; + message: string; +} + +const TRANSPORT_MESSAGES = new Set([ + 'Failed to fetch', + 'Server returned an unreadable response.', +]); + +export function visibleNotice(notices: Notices): string { + return notices.action || notices.connection; +} + +export function dismissNotice(notices: Notices): Notices { + if (notices.action) return { ...notices, action: '' }; + if (notices.connection) return { ...notices, connection: '' }; + return notices; +} + +export function applyRefreshResult( + current: Notices, + result: { ok: true } | PollFailure, +): Notices { + if (result.ok) + return current.connection ? { ...current, connection: '' } : current; + if (result.status === 401) return current; + if (current.connection === result.message) return current; + return { ...current, connection: result.message }; +} + +export function applyCaptureResult( + current: Notices, + result: { ok: true } | PollFailure, +): Notices { + if (result.ok) + return current.connection ? { ...current, connection: '' } : current; + if (result.status === 401) return current; + if ( + result.status === 502 || + result.status === 504 || + TRANSPORT_MESSAGES.has(result.message) + ) { + if (current.connection === result.message) return current; + return { ...current, connection: result.message }; + } + if (current.action === result.message) return current; + return { ...current, action: result.message }; +} diff --git a/tests/poll-notice.test.ts b/tests/poll-notice.test.ts new file mode 100644 index 0000000..4989f17 --- /dev/null +++ b/tests/poll-notice.test.ts @@ -0,0 +1,80 @@ +import { expect, it } from 'vitest'; +import { + applyCaptureResult, + applyRefreshResult, + dismissNotice, + visibleNotice, + type Notices, +} from '../src/client/poll-notice'; + +const saved: Notices = { connection: '', action: 'Could not save.' }; + +it('clears a recovered connection error without wiping an action error', () => { + const lost = applyRefreshResult(saved, { + ok: false, + message: 'Server returned an unreadable response.', + status: 502, + }); + expect(lost).toEqual({ + connection: 'Server returned an unreadable response.', + action: 'Could not save.', + }); + expect(visibleNotice(lost)).toBe('Could not save.'); + + const recovered = applyRefreshResult(lost, { ok: true }); + expect(recovered).toEqual(saved); + expect( + visibleNotice( + applyRefreshResult( + { connection: 'Failed to fetch', action: '' }, + { ok: true }, + ), + ), + ).toBe(''); +}); + +it('keeps an unauthorized poll from replacing the connection notice', () => { + const current: Notices = { connection: 'Failed to fetch', action: '' }; + expect( + applyRefreshResult(current, { + ok: false, + status: 401, + message: 'Enter your owner access token to unlock OpenDots.', + }), + ).toBe(current); +}); + +it('treats a capture transport failure as a connection notice and leaves save errors in place', () => { + const lost = applyCaptureResult(saved, { + ok: false, + message: 'Failed to fetch', + }); + expect(lost.connection).toBe('Failed to fetch'); + expect(lost.action).toBe('Could not save.'); + expect(applyCaptureResult(lost, { ok: true })).toEqual(saved); + + const specific = applyCaptureResult( + { connection: 'Failed to fetch', action: 'Could not save.' }, + { ok: false, status: 500, message: 'Thread not found.' }, + ); + expect(specific.connection).toBe('Failed to fetch'); + expect(specific.action).toBe('Thread not found.'); + expect(applyRefreshResult(specific, { ok: true }).action).toBe( + 'Thread not found.', + ); +}); + +it('dismisses the visible notice and leaves the other one', () => { + const both: Notices = { + connection: 'Failed to fetch', + action: 'Could not save.', + }; + expect(dismissNotice(both)).toEqual({ + connection: 'Failed to fetch', + action: '', + }); + expect(dismissNotice({ connection: 'Failed to fetch', action: '' })).toEqual({ + connection: '', + action: '', + }); +}); From 82edc2258be967e60f888fb1ed66483601453000 Mon Sep 17 00:00:00 2001 From: Charan Rathore <180254320+charan-rathore@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:26:24 +0530 Subject: [PATCH 2/2] fix: keep refresh and capture poll errors separate and treat status-less failures as transport Signed-off-by: Charan Rathore <180254320+charan-rathore@users.noreply.github.com> --- src/client/App.tsx | 3 +- src/client/poll-notice.ts | 44 +++++++++++---------- tests/poll-notice.test.ts | 82 ++++++++++++++++++++++++++------------- 3 files changed, 82 insertions(+), 47 deletions(-) diff --git a/src/client/App.tsx b/src/client/App.tsx index 456047b..8f1861b 100644 --- a/src/client/App.tsx +++ b/src/client/App.tsx @@ -108,7 +108,8 @@ export function App() { }; const [notices, setNotices] = useState({ - connection: '', + refresh: '', + capture: '', action: '', }); const error = visibleNotice(notices); diff --git a/src/client/poll-notice.ts b/src/client/poll-notice.ts index b049c07..c2400f0 100644 --- a/src/client/poll-notice.ts +++ b/src/client/poll-notice.ts @@ -1,5 +1,6 @@ export interface Notices { - connection: string; + refresh: string; + capture: string; action: string; } @@ -9,18 +10,27 @@ export interface PollFailure { message: string; } -const TRANSPORT_MESSAGES = new Set([ - 'Failed to fetch', - 'Server returned an unreadable response.', -]); +const TRANSPORT_MESSAGES = new Set(['Server returned an unreadable response.']); + +// A failure that carries no HTTP status did not come from the API, so it is a +// network failure (fetch throws a TypeError whose text differs by browser). +function isTransportFailure(result: PollFailure): boolean { + return ( + result.status === undefined || + result.status === 502 || + result.status === 504 || + TRANSPORT_MESSAGES.has(result.message) + ); +} export function visibleNotice(notices: Notices): string { - return notices.action || notices.connection; + return notices.action || notices.refresh || notices.capture; } export function dismissNotice(notices: Notices): Notices { if (notices.action) return { ...notices, action: '' }; - if (notices.connection) return { ...notices, connection: '' }; + if (notices.refresh) return { ...notices, refresh: '' }; + if (notices.capture) return { ...notices, capture: '' }; return notices; } @@ -28,27 +38,21 @@ export function applyRefreshResult( current: Notices, result: { ok: true } | PollFailure, ): Notices { - if (result.ok) - return current.connection ? { ...current, connection: '' } : current; + if (result.ok) return current.refresh ? { ...current, refresh: '' } : current; if (result.status === 401) return current; - if (current.connection === result.message) return current; - return { ...current, connection: result.message }; + if (current.refresh === result.message) return current; + return { ...current, refresh: result.message }; } export function applyCaptureResult( current: Notices, result: { ok: true } | PollFailure, ): Notices { - if (result.ok) - return current.connection ? { ...current, connection: '' } : current; + if (result.ok) return current.capture ? { ...current, capture: '' } : current; if (result.status === 401) return current; - if ( - result.status === 502 || - result.status === 504 || - TRANSPORT_MESSAGES.has(result.message) - ) { - if (current.connection === result.message) return current; - return { ...current, connection: result.message }; + if (isTransportFailure(result)) { + if (current.capture === result.message) return current; + return { ...current, capture: result.message }; } if (current.action === result.message) return current; return { ...current, action: result.message }; diff --git a/tests/poll-notice.test.ts b/tests/poll-notice.test.ts index 4989f17..a87e867 100644 --- a/tests/poll-notice.test.ts +++ b/tests/poll-notice.test.ts @@ -7,34 +7,31 @@ import { type Notices, } from '../src/client/poll-notice'; -const saved: Notices = { connection: '', action: 'Could not save.' }; +const none: Notices = { refresh: '', capture: '', action: '' }; +const saved: Notices = { ...none, action: 'Could not save.' }; -it('clears a recovered connection error without wiping an action error', () => { +it('clears a recovered refresh error without wiping an action error', () => { const lost = applyRefreshResult(saved, { ok: false, message: 'Server returned an unreadable response.', status: 502, }); expect(lost).toEqual({ - connection: 'Server returned an unreadable response.', + refresh: 'Server returned an unreadable response.', + capture: '', action: 'Could not save.', }); expect(visibleNotice(lost)).toBe('Could not save.'); - - const recovered = applyRefreshResult(lost, { ok: true }); - expect(recovered).toEqual(saved); + expect(applyRefreshResult(lost, { ok: true })).toEqual(saved); expect( visibleNotice( - applyRefreshResult( - { connection: 'Failed to fetch', action: '' }, - { ok: true }, - ), + applyRefreshResult({ ...none, refresh: 'Failed to fetch' }, { ok: true }), ), ).toBe(''); }); it('keeps an unauthorized poll from replacing the connection notice', () => { - const current: Notices = { connection: 'Failed to fetch', action: '' }; + const current: Notices = { ...none, refresh: 'Failed to fetch' }; expect( applyRefreshResult(current, { ok: false, @@ -44,37 +41,70 @@ it('keeps an unauthorized poll from replacing the connection notice', () => { ).toBe(current); }); -it('treats a capture transport failure as a connection notice and leaves save errors in place', () => { +it('treats a capture transport failure as a capture notice and leaves save errors in place', () => { const lost = applyCaptureResult(saved, { ok: false, message: 'Failed to fetch', }); - expect(lost.connection).toBe('Failed to fetch'); + expect(lost.capture).toBe('Failed to fetch'); expect(lost.action).toBe('Could not save.'); expect(applyCaptureResult(lost, { ok: true })).toEqual(saved); const specific = applyCaptureResult( - { connection: 'Failed to fetch', action: 'Could not save.' }, + { ...none, capture: 'Failed to fetch', action: 'Could not save.' }, { ok: false, status: 500, message: 'Thread not found.' }, ); - expect(specific.connection).toBe('Failed to fetch'); + expect(specific.capture).toBe('Failed to fetch'); expect(specific.action).toBe('Thread not found.'); expect(applyRefreshResult(specific, { ok: true }).action).toBe( 'Thread not found.', ); }); -it('dismisses the visible notice and leaves the other one', () => { - const both: Notices = { - connection: 'Failed to fetch', - action: 'Could not save.', - }; - expect(dismissNotice(both)).toEqual({ - connection: 'Failed to fetch', - action: '', +it('treats any failure without an HTTP status as a transport failure', () => { + for (const message of [ + 'Failed to fetch', + 'NetworkError when attempting to fetch resource.', + 'Load failed', + ]) { + const lost = applyCaptureResult(none, { ok: false, message }); + expect(lost).toEqual({ ...none, capture: message }); + expect(applyCaptureResult(lost, { ok: true })).toEqual(none); + } +}); + +it('keeps a failing refresh poll visible while capture polls succeed', () => { + const failing = applyRefreshResult(none, { + ok: false, + status: 503, + message: 'Service unavailable.', }); - expect(dismissNotice({ connection: 'Failed to fetch', action: '' })).toEqual({ - connection: '', - action: '', + const afterCapture = applyCaptureResult(failing, { ok: true }); + expect(afterCapture.refresh).toBe('Service unavailable.'); + expect(visibleNotice(afterCapture)).toBe('Service unavailable.'); + expect(applyRefreshResult(afterCapture, { ok: true })).toEqual(none); +}); + +it('keeps a failing capture poll visible while refresh polls succeed', () => { + const failing = applyCaptureResult(none, { + ok: false, + status: 504, + message: 'Gateway timeout.', }); + const afterRefresh = applyRefreshResult(failing, { ok: true }); + expect(afterRefresh.capture).toBe('Gateway timeout.'); + expect(applyCaptureResult(afterRefresh, { ok: true })).toEqual(none); +}); + +it('dismisses the visible notice and leaves the others', () => { + const all: Notices = { + refresh: 'Service unavailable.', + capture: 'Failed to fetch', + action: 'Could not save.', + }; + const first = dismissNotice(all); + expect(first).toEqual({ ...all, action: '' }); + const second = dismissNotice(first); + expect(second).toEqual({ ...all, action: '', refresh: '' }); + expect(dismissNotice(second)).toEqual(none); });