From 77d9de42f477e2ed862423b8c9751c4112260d1d Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Mon, 3 Aug 2026 06:56:27 -0600 Subject: [PATCH 1/2] fix: allow use24HourTime persist and soften reticulum proxy restart errors Allowlist the Appearance 24-hour setting for SQLite, and return expected sidecar-down proxy failures as envelopes so Electron does not log stack restart races as handler errors. --- src/main/database.test.ts | 1 + src/main/index.contract.test.ts | 5 + src/main/index.ts | 1 + src/main/ipc/reticulum-handlers.test.ts | 15 ++- src/main/ipc/reticulum-handlers.ts | 53 ++++++---- src/preload/index.ts | 99 +++++++++++++------ .../reticulum/reticulumSidecarReads.test.ts | 8 +- .../lib/reticulum/reticulumSidecarReads.ts | 10 +- src/shared/reticulumProxyIpcError.test.ts | 41 ++++++++ src/shared/reticulumProxyIpcError.ts | 46 +++++++++ 10 files changed, 213 insertions(+), 66 deletions(-) create mode 100644 src/shared/reticulumProxyIpcError.test.ts create mode 100644 src/shared/reticulumProxyIpcError.ts diff --git a/src/main/database.test.ts b/src/main/database.test.ts index 78921d99a..62d3bc7a5 100644 --- a/src/main/database.test.ts +++ b/src/main/database.test.ts @@ -410,6 +410,7 @@ describe('app_settings table + message retention defaults (schema sync)', () => expect(INDEX_SOURCE).toContain('meshcoreMessageRetentionEnabled'); expect(INDEX_SOURCE).toContain('meshcoreMessageRetentionCount'); expect(INDEX_SOURCE).toContain('reduceMotion'); + expect(INDEX_SOURCE).toContain('use24HourTime'); expect(INDEX_SOURCE).toContain('meshcoreRoomSync:'); expect(INDEX_SOURCE).toContain('meshcoreRoomLastPost:'); expect(INDEX_SOURCE).toContain('meshcoreRoomCredential:'); diff --git a/src/main/index.contract.test.ts b/src/main/index.contract.test.ts index 37d349308..0c6710c47 100644 --- a/src/main/index.contract.test.ts +++ b/src/main/index.contract.test.ts @@ -149,6 +149,7 @@ describe('Persistent app settings IPC (source contract)', () => { expect(INDEX_SOURCE).toMatch(/key not allowed/); expect(INDEX_SOURCE).toContain("'meshtasticLastRfSelfNodeId'"); expect(INDEX_SOURCE).toContain("'meshcoreLastSelfNodeId'"); + expect(INDEX_SOURCE).toContain("'use24HourTime'"); expect(INDEX_SOURCE).toContain('meshtasticRemoteAdminKey:'); expect(INDEX_SOURCE).toContain('meshcoreRoomSync:'); expect(INDEX_SOURCE).toContain('meshcoreRoomLastPost:'); @@ -309,6 +310,10 @@ describe('Reticulum sidecar IPC handlers (source contract)', () => { expect(RETICULUM_HANDLERS_SOURCE).toContain("ipcMain.handle('reticulum:getStatus'"); expect(RETICULUM_HANDLERS_SOURCE).toContain("'reticulum:syncInterfaceIssueScope'"); expect(RETICULUM_HANDLERS_SOURCE).toContain("ipcMain.handle('reticulum:proxyGet'"); + expect(RETICULUM_HANDLERS_SOURCE).toContain('settleReticulumProxyFailure'); + expect(RETICULUM_HANDLERS_SOURCE).toContain('reticulumProxyIpcErrorEnvelope'); + expect(PRELOAD_SOURCE).toContain('unwrapReticulumProxy'); + expect(PRELOAD_SOURCE).toContain('throwIfReticulumProxyIpcError'); expect(PRELOAD_SOURCE).toContain("'/api/v1/rrc/hubs'"); expect(PRELOAD_SOURCE).toContain('rrc:'); expect(RETICULUM_HANDLERS_SOURCE).toContain("ipcMain.handle('reticulum:proxyPost'"); diff --git a/src/main/index.ts b/src/main/index.ts index f2e480f11..4a00ca815 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -3524,6 +3524,7 @@ const APP_SETTINGS_ALLOWED_KEYS: ReadonlySet = new Set([ 'meshcoreLastSelfNodeId', 'storeForwardAutoFetchHistory', 'reduceMotion', + 'use24HourTime', 'alwaysShowMessageActions', 'reticulumAutostart', 'reticulumRmapAnnounceIntervalMin', diff --git a/src/main/ipc/reticulum-handlers.test.ts b/src/main/ipc/reticulum-handlers.test.ts index 3d7a1aa65..5947e2591 100644 --- a/src/main/ipc/reticulum-handlers.test.ts +++ b/src/main/ipc/reticulum-handlers.test.ts @@ -294,11 +294,20 @@ describe('registerReticulumIpcHandlers', () => { expect(ensureManager).not.toHaveBeenCalled(); }); - it('proxyGet rethrows failures from the manager', async () => { - manager.proxyGet.mockRejectedValueOnce(new Error('sidecar not running')); + it('proxyGet returns a soft-failure envelope for expected sidecar-down races', async () => { + manager.proxyGet.mockRejectedValueOnce(new Error('Reticulum sidecar is not running')); + const result = await handlers.get('reticulum:proxyGet')?.(event, '/api/v1/diagnostics'); + expect(result).toEqual({ + __reticulumProxyError: true, + message: 'Reticulum sidecar is not running', + }); + }); + + it('proxyGet rethrows unexpected manager failures', async () => { + manager.proxyGet.mockRejectedValueOnce(new Error('EACCES permission denied')); await expect( handlers.get('reticulum:proxyGet')?.(event, '/api/v1/diagnostics'), - ).rejects.toThrow('sidecar not running'); + ).rejects.toThrow('EACCES permission denied'); }); it('proxyPost forwards path and body to manager.proxyPost', async () => { diff --git a/src/main/ipc/reticulum-handlers.ts b/src/main/ipc/reticulum-handlers.ts index 2c84e386b..83a6572a5 100644 --- a/src/main/ipc/reticulum-handlers.ts +++ b/src/main/ipc/reticulum-handlers.ts @@ -6,6 +6,11 @@ import type { ReticulumSidecarStatus, } from '../../shared/reticulum-types'; import { canonicalizeReticulumDestinationHash } from '../../shared/reticulumDestinationHash'; +import { + isExpectedReticulumProxyErrorMessage, + type ReticulumProxyIpcErrorEnvelope, + reticulumProxyIpcErrorEnvelope, +} from '../../shared/reticulumProxyIpcError'; import { MS_PER_MINUTE } from '../../shared/timeConstants'; import { createIpcRateLimiter } from '../ipcRateLimit'; import { sanitizeLogMessage } from '../log-service'; @@ -45,18 +50,6 @@ export interface ReticulumIpcDeps { getMainWindow: () => BrowserWindow | null; } -function isExpectedReticulumProxyError(message: string): boolean { - const lower = message.toLowerCase(); - return ( - lower.includes('not running') || - message.includes('404') || - lower.includes('fetch failed') || - lower.includes('aborted') || - lower.includes('timeout') || - lower.includes('rate limit exceeded') - ); -} - function parseReticulumStartOptions(opts: unknown): ReticulumSidecarStartOptions { if (opts == null) return {}; if (typeof opts !== 'object' || Array.isArray(opts)) { @@ -71,11 +64,29 @@ function parseReticulumStartOptions(opts: unknown): ReticulumSidecarStartOptions function logReticulumProxyFailure(method: string, err: unknown, apiPath?: string): void { const message = err instanceof Error ? err.message : String(err); - const log = isExpectedReticulumProxyError(message) ? console.debug : console.error; + const log = isExpectedReticulumProxyErrorMessage(message) ? console.debug : console.error; const pathSuffix = apiPath ? ` path=${apiPath}` : ''; log(`[ReticulumIPC] ${method} failed${pathSuffix}:`, sanitizeLogMessage(message)); } +/** + * Expected restart/transient failures: return an envelope (preload rethrows) so + * Electron does not emit `[error] Error occurred in handler for 'reticulum:proxy*'`. + * Unexpected failures still throw. + */ +function settleReticulumProxyFailure( + method: string, + err: unknown, + apiPath?: string, +): ReticulumProxyIpcErrorEnvelope { + logReticulumProxyFailure(method, err, apiPath); + const message = err instanceof Error ? err.message : String(err); + if (isExpectedReticulumProxyErrorMessage(message)) { + return reticulumProxyIpcErrorEnvelope(sanitizeLogMessage(message)); + } + throw err; +} + function assertProxyApiPath(apiPath: unknown): string { if (typeof apiPath !== 'string') { throw new Error('Reticulum proxy path must be a string'); @@ -175,8 +186,8 @@ export function registerReticulumIpcHandlers(deps: ReticulumIpcDeps): void { const m = ensureManager(); return await m.proxyGet(pathArg); } catch (err) { - logReticulumProxyFailure('proxyGet', err, pathArg); - throw err; + // catch-no-log-ok settleReticulumProxyFailure logs expected failures / rethrows unexpected + return settleReticulumProxyFailure('proxyGet', err, pathArg); } }); @@ -193,8 +204,8 @@ export function registerReticulumIpcHandlers(deps: ReticulumIpcDeps): void { const m = ensureManager(); return await m.proxyPost(pathArg, body); } catch (err) { - logReticulumProxyFailure('proxyPost', err, pathArg); - throw err; + // catch-no-log-ok settleReticulumProxyFailure logs expected failures / rethrows unexpected + return settleReticulumProxyFailure('proxyPost', err, pathArg); } }); @@ -223,8 +234,8 @@ export function registerReticulumIpcHandlers(deps: ReticulumIpcDeps): void { const m = ensureManager(); return await m.proxyPut(pathArg, body); } catch (err) { - logReticulumProxyFailure('proxyPut', err, pathArg); - throw err; + // catch-no-log-ok settleReticulumProxyFailure logs expected failures / rethrows unexpected + return settleReticulumProxyFailure('proxyPut', err, pathArg); } }); @@ -236,8 +247,8 @@ export function registerReticulumIpcHandlers(deps: ReticulumIpcDeps): void { const m = ensureManager(); return await m.proxyDelete(pathArg); } catch (err) { - logReticulumProxyFailure('proxyDelete', err, pathArg); - throw err; + // catch-no-log-ok settleReticulumProxyFailure logs expected failures / rethrows unexpected + return settleReticulumProxyFailure('proxyDelete', err, pathArg); } }); diff --git a/src/preload/index.ts b/src/preload/index.ts index 95e1274e4..775d7b1a4 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -27,10 +27,16 @@ import type { ReticulumSidecarStartOptions, ReticulumSidecarStatus, } from '../shared/reticulum-types'; +import { throwIfReticulumProxyIpcError } from '../shared/reticulumProxyIpcError'; import type { TAKClientInfo, TAKServerStatus, TAKSettings } from '../shared/tak-types'; export type { NobleBleDevice, NobleBleSessionId, SerialPort }; +/** Unwrap reticulum proxy soft-failure envelopes so renderer catch paths stay the same. */ +async function unwrapReticulumProxy(result: Promise): Promise { + return throwIfReticulumProxyIpcError(await result) as T; +} + contextBridge.exposeInMainWorld('electronAPI', { // ─── Database operations ──────────────────────────────────────── db: { @@ -1071,13 +1077,13 @@ contextBridge.exposeInMainWorld('electronAPI', { syncInterfaceIssueScope: (enabledInterfaceNames: string[]): Promise => ipcRenderer.invoke('reticulum:syncInterfaceIssueScope', enabledInterfaceNames), proxyGet: (apiPath: string): Promise => - ipcRenderer.invoke('reticulum:proxyGet', apiPath), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', apiPath)), proxyPost: (apiPath: string, body: unknown): Promise => - ipcRenderer.invoke('reticulum:proxyPost', apiPath, body), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', apiPath, body)), proxyPut: (apiPath: string, body: unknown): Promise => - ipcRenderer.invoke('reticulum:proxyPut', apiPath, body), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPut', apiPath, body)), proxyDelete: (apiPath: string): Promise => - ipcRenderer.invoke('reticulum:proxyDelete', apiPath), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyDelete', apiPath)), factoryReset: (): Promise => ipcRenderer.invoke('reticulum:factoryReset'), readDefaultConfigFile: (): Promise<{ path: string | null; content: string | null }> => ipcRenderer.invoke('reticulum:readDefaultConfigFile'), @@ -1105,49 +1111,67 @@ contextBridge.exposeInMainWorld('electronAPI', { return () => ipcRenderer.off('reticulum:status', handler); }, rrc: { - listHubs: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rrc/hubs'), + listHubs: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rrc/hubs')), upsertHub: (opts: { dest_hash: string; label?: string; favorited?: boolean }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/hubs', opts), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/hubs', opts)), setFavorite: (destHash: string, favorited: boolean) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/hubs/favorite', { - dest_hash: destHash, - favorited, - }), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/hubs/favorite', { + dest_hash: destHash, + favorited, + }), + ), connect: (opts: { dest_hash: string; nickname?: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/connect', opts), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/connect', opts), + ), disconnect: (opts?: { dest_hash?: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/disconnect', opts ?? {}), - getStatus: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rrc/status'), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/disconnect', opts ?? {}), + ), + getStatus: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rrc/status')), join: (opts: { hub_dest_hash: string; room: string; key?: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/join', opts), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/join', opts)), part: (opts: { hub_dest_hash: string; room: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/part', opts), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/part', opts)), send: (opts: { hub_dest_hash: string; room?: string; body: string; type?: string; dst_hash?: string; - }) => ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/send', opts), + }) => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/send', opts)), setNickname: (opts: { nickname: string; hub_dest_hash?: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/nick', opts), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rrc/nick', opts)), getRooms: (hubDestHash?: string) => { const q = hubDestHash?.trim() ? `?hub_dest_hash=${encodeURIComponent(hubDestHash.trim().toLowerCase())}` : ''; - return ipcRenderer.invoke('reticulum:proxyGet', `/api/v1/rrc/rooms${q}`); + return unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyGet', `/api/v1/rrc/rooms${q}`), + ); }, }, rnsh: { connect: (opts: { destination_hash: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/connect', opts), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/connect', opts), + ), input: (opts: { session_id: string; data: string; encoding?: 'base64' }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/input', opts), + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/input', opts)), resize: (opts: { session_id: string; rows?: number; cols?: number }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/resize', opts), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/resize', opts), + ), disconnect: (opts: { session_id: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/disconnect', opts), - getStatus: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rnsh/status'), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rnsh/disconnect', opts), + ), + getStatus: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rnsh/status')), }, rncp: { send: (opts: { destination_hash: string; path: string }) => @@ -1155,13 +1179,21 @@ contextBridge.exposeInMainWorld('electronAPI', { fetch: (opts: { destination_hash: string; remote_path: string; save_path?: string }) => ipcRenderer.invoke('reticulum:rncpFetch', opts), cancel: (opts: { transfer_id: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/cancel', opts), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/cancel', opts), + ), accept: (opts: { transfer_id: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/accept', opts), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/accept', opts), + ), reject: (opts: { transfer_id: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/reject', opts), - getStatus: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rncp/status'), - getListener: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rncp/listener'), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/reject', opts), + ), + getStatus: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rncp/status')), + getListener: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/rncp/listener')), setListener: (opts: { enabled: boolean; save_dir?: string; @@ -1172,7 +1204,9 @@ contextBridge.exposeInMainWorld('electronAPI', { blocked?: string[]; }) => ipcRenderer.invoke('reticulum:setRncpListener', opts), announce: (): Promise<{ ok: boolean; error?: string }> => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/announce', {}), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/rncp/announce', {}), + ), showOpenFileDialog: (): Promise<{ canceled: boolean; path: string | null }> => ipcRenderer.invoke('reticulum:showRncpOpenFileDialog'), showSaveDirectoryDialog: (): Promise<{ canceled: boolean; path: string | null }> => @@ -1182,8 +1216,11 @@ contextBridge.exposeInMainWorld('electronAPI', { }, remote: { pathCapability: (opts: { destination_hash: string }) => - ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/remote/path-capability', opts), - getIdentity: () => ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/remote/identity'), + unwrapReticulumProxy( + ipcRenderer.invoke('reticulum:proxyPost', '/api/v1/remote/path-capability', opts), + ), + getIdentity: () => + unwrapReticulumProxy(ipcRenderer.invoke('reticulum:proxyGet', '/api/v1/remote/identity')), }, }, diff --git a/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts b/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts index 7212a78cc..ad453f141 100644 --- a/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts +++ b/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts @@ -175,13 +175,15 @@ describe('reticulumSidecarReads', () => { it('fetchReticulumRmapDiscovered throws on unexpected proxy errors', async () => { getStatus.mockResolvedValue({ running: true, port: 1, pid: 1 }); - proxyGet.mockRejectedValue(new Error('sidecar timeout')); - await expect(fetchReticulumRmapDiscovered()).rejects.toThrow('sidecar timeout'); + proxyGet.mockRejectedValue(new Error('EACCES permission denied')); + await expect(fetchReticulumRmapDiscovered()).rejects.toThrow('EACCES permission denied'); }); it('fetchReticulumRmapDiscovered returns empty on expected proxy errors', async () => { getStatus.mockResolvedValue({ running: true, port: 1, pid: 1 }); - proxyGet.mockRejectedValue(new Error('Reticulum sidecar is not running')); + proxyGet.mockRejectedValueOnce(new Error('Reticulum sidecar is not running')); + await expect(fetchReticulumRmapDiscovered()).resolves.toEqual([]); + proxyGet.mockRejectedValueOnce(new Error('sidecar timeout')); await expect(fetchReticulumRmapDiscovered()).resolves.toEqual([]); }); diff --git a/src/renderer/lib/reticulum/reticulumSidecarReads.ts b/src/renderer/lib/reticulum/reticulumSidecarReads.ts index 914b46911..550efdf54 100644 --- a/src/renderer/lib/reticulum/reticulumSidecarReads.ts +++ b/src/renderer/lib/reticulum/reticulumSidecarReads.ts @@ -6,6 +6,7 @@ import { reticulumHashToNodeId, } from '@/renderer/lib/reticulum/destHash'; import type { ReticulumRmapDiscoveredWireRow } from '@/shared/reticulum-types'; +import { isExpectedReticulumProxyErrorMessage } from '@/shared/reticulumProxyIpcError'; export interface ReticulumIdentityStatus { configured: boolean; @@ -57,14 +58,7 @@ export function isReticulumSidecarRateLimitError(err: unknown): boolean { } export function isReticulumSidecarExpectedProxyError(err: unknown): boolean { - const msg = errLikeToLogString(err).toLowerCase(); - return ( - isReticulumSidecarNotRunningError(err) || - isReticulumSidecar404Error(err) || - isReticulumSidecarRateLimitError(err) || - msg.includes('fetch failed') || - msg.includes('aborted') - ); + return isExpectedReticulumProxyErrorMessage(errLikeToLogString(err)); } export interface ReticulumSidecarInterfaceRow { diff --git a/src/shared/reticulumProxyIpcError.test.ts b/src/shared/reticulumProxyIpcError.test.ts new file mode 100644 index 000000000..d9d35353c --- /dev/null +++ b/src/shared/reticulumProxyIpcError.test.ts @@ -0,0 +1,41 @@ +// @vitest-environment node +import { describe, expect, it } from 'vitest'; + +import { + isExpectedReticulumProxyErrorMessage, + isReticulumProxyIpcErrorEnvelope, + reticulumProxyIpcErrorEnvelope, + throwIfReticulumProxyIpcError, +} from './reticulumProxyIpcError'; + +describe('reticulumProxyIpcError', () => { + it.each([ + 'Reticulum sidecar is not running', + 'fetch failed', + 'TypeError: fetch failed', + 'aborted', + 'request timeout', + 'rate limit exceeded', + 'HTTP 404', + ])('treats %j as expected', (message) => { + expect(isExpectedReticulumProxyErrorMessage(message)).toBe(true); + }); + + it('rejects unrelated errors', () => { + expect(isExpectedReticulumProxyErrorMessage('EACCES permission denied')).toBe(false); + }); + + it('builds and detects envelopes', () => { + const env = reticulumProxyIpcErrorEnvelope('Reticulum sidecar is not running'); + expect(isReticulumProxyIpcErrorEnvelope(env)).toBe(true); + expect(isReticulumProxyIpcErrorEnvelope({ ok: true })).toBe(false); + expect(isReticulumProxyIpcErrorEnvelope(null)).toBe(false); + }); + + it('throwIfReticulumProxyIpcError rethrows envelopes and passes through values', () => { + expect(throwIfReticulumProxyIpcError({ peers: [] })).toEqual({ peers: [] }); + expect(() => + throwIfReticulumProxyIpcError(reticulumProxyIpcErrorEnvelope('fetch failed')), + ).toThrow('fetch failed'); + }); +}); diff --git a/src/shared/reticulumProxyIpcError.ts b/src/shared/reticulumProxyIpcError.ts new file mode 100644 index 000000000..05dc6b2ad --- /dev/null +++ b/src/shared/reticulumProxyIpcError.ts @@ -0,0 +1,46 @@ +/** + * Expected Reticulum proxy failures (sidecar restart / not running / transient + * fetch) are returned as this envelope instead of rejecting the IPC promise. + * Electron logs every rejected `ipcMain.handle` as `[error] Error occurred in + * handler…` — returning a value keeps stack-restart races at debug noise while + * preload rethrows so renderer try/catch stays unchanged. + */ +export const RETICULUM_PROXY_IPC_ERROR_TAG = '__reticulumProxyError' as const; + +export interface ReticulumProxyIpcErrorEnvelope { + readonly [RETICULUM_PROXY_IPC_ERROR_TAG]: true; + readonly message: string; +} + +/** True when message matches transient sidecar/proxy failures (restart races). */ +export function isExpectedReticulumProxyErrorMessage(message: string): boolean { + const lower = message.toLowerCase(); + return ( + lower.includes('not running') || + message.includes('404') || + lower.includes('fetch failed') || + lower.includes('aborted') || + lower.includes('timeout') || + lower.includes('rate limit exceeded') + ); +} + +export function isReticulumProxyIpcErrorEnvelope( + value: unknown, +): value is ReticulumProxyIpcErrorEnvelope { + if (value == null || typeof value !== 'object' || Array.isArray(value)) return false; + const rec = value as Record; + return rec[RETICULUM_PROXY_IPC_ERROR_TAG] === true && typeof rec.message === 'string'; +} + +export function reticulumProxyIpcErrorEnvelope(message: string): ReticulumProxyIpcErrorEnvelope { + return { [RETICULUM_PROXY_IPC_ERROR_TAG]: true, message }; +} + +/** Preload: turn envelope into a thrown Error so renderer catch paths stay the same. */ +export function throwIfReticulumProxyIpcError(value: unknown): unknown { + if (isReticulumProxyIpcErrorEnvelope(value)) { + throw new Error(value.message); + } + return value; +} From 7869f02a4c3acb8fec29af44eda94e8e32ee31b1 Mon Sep 17 00:00:00 2001 From: Joey Stanford Date: Mon, 3 Aug 2026 07:12:57 -0600 Subject: [PATCH 2/2] test: cover proxy soft-failures and tighten expected-error matching Fix the rate-limit source contract after the classifier moved to shared, extend envelope/rethrow coverage to proxyPost/Put/Delete, and prefer structured 404 / AbortError signals over bare message substrings. --- src/main/ipc/reticulum-handlers.test.ts | 34 +++++++++ src/main/ipc/reticulum-handlers.ts | 6 +- ...eticulum-proxy-rate-limit.contract.test.ts | 8 +- .../reticulum/reticulumSidecarReads.test.ts | 2 + .../lib/reticulum/reticulumSidecarReads.ts | 14 +++- src/shared/reticulumProxyIpcError.test.ts | 27 ++++++- src/shared/reticulumProxyIpcError.ts | 75 ++++++++++++++++--- 7 files changed, 146 insertions(+), 20 deletions(-) diff --git a/src/main/ipc/reticulum-handlers.test.ts b/src/main/ipc/reticulum-handlers.test.ts index 5947e2591..9a9847356 100644 --- a/src/main/ipc/reticulum-handlers.test.ts +++ b/src/main/ipc/reticulum-handlers.test.ts @@ -310,6 +310,40 @@ describe('registerReticulumIpcHandlers', () => { ).rejects.toThrow('EACCES permission denied'); }); + it.each([ + ['proxyPost', '/api/v1/lxmf/send', { text: 'hi' }] as const, + ['proxyPut', '/api/v1/interfaces/tcp', { enabled: true }] as const, + ['proxyDelete', '/api/v1/interfaces/tcp', undefined] as const, + ])( + '%s returns a soft-failure envelope for expected sidecar-down races', + async (method, path, body) => { + manager[method].mockRejectedValueOnce(new Error('Reticulum sidecar is not running')); + const channel = `reticulum:${method}` as const; + const result = + body === undefined + ? await handlers.get(channel)?.(event, path) + : await handlers.get(channel)?.(event, path, body); + expect(result).toEqual({ + __reticulumProxyError: true, + message: 'Reticulum sidecar is not running', + }); + }, + ); + + it.each([ + ['proxyPost', '/api/v1/lxmf/send', { text: 'hi' }] as const, + ['proxyPut', '/api/v1/interfaces/tcp', { enabled: true }] as const, + ['proxyDelete', '/api/v1/interfaces/tcp', undefined] as const, + ])('%s rethrows unexpected manager failures', async (method, path, body) => { + manager[method].mockRejectedValueOnce(new Error('EACCES permission denied')); + const channel = `reticulum:${method}` as const; + const invoke = + body === undefined + ? handlers.get(channel)?.(event, path) + : handlers.get(channel)?.(event, path, body); + await expect(invoke).rejects.toThrow('EACCES permission denied'); + }); + it('proxyPost forwards path and body to manager.proxyPost', async () => { const body = { destination_hash: 'aa'.repeat(16), text: 'hi' }; await handlers.get('reticulum:proxyPost')?.(event, '/api/v1/lxmf/send', body); diff --git a/src/main/ipc/reticulum-handlers.ts b/src/main/ipc/reticulum-handlers.ts index 83a6572a5..fc50be9d8 100644 --- a/src/main/ipc/reticulum-handlers.ts +++ b/src/main/ipc/reticulum-handlers.ts @@ -7,7 +7,7 @@ import type { } from '../../shared/reticulum-types'; import { canonicalizeReticulumDestinationHash } from '../../shared/reticulumDestinationHash'; import { - isExpectedReticulumProxyErrorMessage, + isExpectedReticulumProxyError, type ReticulumProxyIpcErrorEnvelope, reticulumProxyIpcErrorEnvelope, } from '../../shared/reticulumProxyIpcError'; @@ -64,7 +64,7 @@ function parseReticulumStartOptions(opts: unknown): ReticulumSidecarStartOptions function logReticulumProxyFailure(method: string, err: unknown, apiPath?: string): void { const message = err instanceof Error ? err.message : String(err); - const log = isExpectedReticulumProxyErrorMessage(message) ? console.debug : console.error; + const log = isExpectedReticulumProxyError(err) ? console.debug : console.error; const pathSuffix = apiPath ? ` path=${apiPath}` : ''; log(`[ReticulumIPC] ${method} failed${pathSuffix}:`, sanitizeLogMessage(message)); } @@ -81,7 +81,7 @@ function settleReticulumProxyFailure( ): ReticulumProxyIpcErrorEnvelope { logReticulumProxyFailure(method, err, apiPath); const message = err instanceof Error ? err.message : String(err); - if (isExpectedReticulumProxyErrorMessage(message)) { + if (isExpectedReticulumProxyError(err)) { return reticulumProxyIpcErrorEnvelope(sanitizeLogMessage(message)); } throw err; diff --git a/src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts b/src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts index d88fdf115..e7d4454a6 100644 --- a/src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts +++ b/src/main/ipc/reticulum-proxy-rate-limit.contract.test.ts @@ -17,7 +17,13 @@ describe('reticulum proxy rate limit + 100k peer ceilings (source contract)', () it('caps shared proxy IPC at 300/min and treats rate-limit as expected', () => { expect(HANDLERS_SOURCE).toMatch(/max:\s*300/); expect(HANDLERS_SOURCE).toContain("label: 'reticulum:proxy'"); - expect(HANDLERS_SOURCE).toContain("lower.includes('rate limit exceeded')"); + expect(HANDLERS_SOURCE).toContain('isExpectedReticulumProxyError'); + expect(HANDLERS_SOURCE).toContain("from '../../shared/reticulumProxyIpcError'"); + const sharedSource = readFileSync( + join(__dirname, '../../shared/reticulumProxyIpcError.ts'), + 'utf-8', + ); + expect(sharedSource).toContain("lower.includes('rate limit exceeded')"); }); it('applies the shared proxy rate limit to picker-gated RNCP handlers', () => { diff --git a/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts b/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts index ad453f141..24eb5edec 100644 --- a/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts +++ b/src/renderer/lib/reticulum/reticulumSidecarReads.test.ts @@ -58,6 +58,8 @@ describe('reticulumSidecarReads', () => { expect(isReticulumSidecar404Error(new Error('sidecar GET /api/v1/topology failed: 404'))).toBe( true, ); + expect(isReticulumSidecar404Error({ status: 404, message: 'missing route' })).toBe(true); + expect(isReticulumSidecar404Error(new Error('payload size 4048 bytes'))).toBe(false); expect( isReticulumSidecarRateLimitError(new Error('reticulum:proxy: rate limit exceeded')), ).toBe(true); diff --git a/src/renderer/lib/reticulum/reticulumSidecarReads.ts b/src/renderer/lib/reticulum/reticulumSidecarReads.ts index 550efdf54..2a37615b5 100644 --- a/src/renderer/lib/reticulum/reticulumSidecarReads.ts +++ b/src/renderer/lib/reticulum/reticulumSidecarReads.ts @@ -6,7 +6,7 @@ import { reticulumHashToNodeId, } from '@/renderer/lib/reticulum/destHash'; import type { ReticulumRmapDiscoveredWireRow } from '@/shared/reticulum-types'; -import { isExpectedReticulumProxyErrorMessage } from '@/shared/reticulumProxyIpcError'; +import { isExpectedReticulumProxyError } from '@/shared/reticulumProxyIpcError'; export interface ReticulumIdentityStatus { configured: boolean; @@ -50,7 +50,15 @@ export function isReticulumSidecarNotRunningError(err: unknown): boolean { } export function isReticulumSidecar404Error(err: unknown): boolean { - return errLikeToLogString(err).includes('404'); + if (err != null && typeof err === 'object') { + const rec = err as Record; + for (const key of ['status', 'statusCode'] as const) { + const raw = rec[key]; + if (raw === 404 || raw === '404') return true; + } + } + // Sidecar manager: `sidecar GET … failed: 404` + return /failed:\s*404\b/i.test(errLikeToLogString(err)); } export function isReticulumSidecarRateLimitError(err: unknown): boolean { @@ -58,7 +66,7 @@ export function isReticulumSidecarRateLimitError(err: unknown): boolean { } export function isReticulumSidecarExpectedProxyError(err: unknown): boolean { - return isExpectedReticulumProxyErrorMessage(errLikeToLogString(err)); + return isExpectedReticulumProxyError(err); } export interface ReticulumSidecarInterfaceRow { diff --git a/src/shared/reticulumProxyIpcError.test.ts b/src/shared/reticulumProxyIpcError.test.ts index d9d35353c..dad0cc092 100644 --- a/src/shared/reticulumProxyIpcError.test.ts +++ b/src/shared/reticulumProxyIpcError.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { + isExpectedReticulumProxyError, isExpectedReticulumProxyErrorMessage, isReticulumProxyIpcErrorEnvelope, reticulumProxyIpcErrorEnvelope, @@ -13,16 +14,34 @@ describe('reticulumProxyIpcError', () => { 'Reticulum sidecar is not running', 'fetch failed', 'TypeError: fetch failed', - 'aborted', + 'The operation was aborted', + 'The operation was aborted due to timeout', 'request timeout', - 'rate limit exceeded', - 'HTTP 404', + 'sidecar timeout', + 'link timed out waiting for proof', + 'reticulum:proxy: rate limit exceeded', + 'sidecar GET /api/v1/topology failed: 404', ])('treats %j as expected', (message) => { expect(isExpectedReticulumProxyErrorMessage(message)).toBe(true); + expect(isExpectedReticulumProxyError(new Error(message))).toBe(true); }); - it('rejects unrelated errors', () => { + it('rejects unrelated errors and unanchored 404/timeout substrings', () => { expect(isExpectedReticulumProxyErrorMessage('EACCES permission denied')).toBe(false); + expect(isExpectedReticulumProxyErrorMessage('channel 1404 unavailable')).toBe(false); + expect(isExpectedReticulumProxyErrorMessage('payload size 4048 bytes')).toBe(false); + expect(isExpectedReticulumProxyErrorMessage('HTTP 404')).toBe(false); + expect(isExpectedReticulumProxyError(new Error('EACCES permission denied'))).toBe(false); + }); + + it('treats structured status/statusCode 404 and Abort/Timeout names as expected', () => { + expect(isExpectedReticulumProxyError({ status: 404, message: 'missing' })).toBe(true); + expect(isExpectedReticulumProxyError({ statusCode: '404' })).toBe(true); + expect(isExpectedReticulumProxyError({ name: 'AbortError', message: 'aborted' })).toBe(true); + expect(isExpectedReticulumProxyError({ name: 'TimeoutError', message: 'took too long' })).toBe( + true, + ); + expect(isExpectedReticulumProxyError({ status: 500, message: 'boom' })).toBe(false); }); it('builds and detects envelopes', () => { diff --git a/src/shared/reticulumProxyIpcError.ts b/src/shared/reticulumProxyIpcError.ts index 05dc6b2ad..8f1bb998d 100644 --- a/src/shared/reticulumProxyIpcError.ts +++ b/src/shared/reticulumProxyIpcError.ts @@ -12,17 +12,74 @@ export interface ReticulumProxyIpcErrorEnvelope { readonly message: string; } -/** True when message matches transient sidecar/proxy failures (restart races). */ +/** HTTP status from sidecar manager errors shaped like `sidecar GET … failed: 404`. */ +function httpStatusFromProxyMessage(message: string): number | null { + const m = /failed:\s*(\d{3})\b/i.exec(message); + if (!m?.[1]) return null; + const status = Number(m[1]); + return Number.isFinite(status) ? status : null; +} + +function readNumericStatusField(err: object): number | null { + const rec = err as Record; + for (const key of ['status', 'statusCode'] as const) { + const raw = rec[key]; + if (typeof raw === 'number' && Number.isFinite(raw)) return raw; + if (typeof raw === 'string' && /^\d{3}$/.test(raw)) return Number(raw); + } + return null; +} + +/** + * True when message matches genuine transient sidecar/proxy failures + * (restart races, AbortSignal timeout, IPC rate limit). Avoids bare `404` / + * `timeout` substrings that can appear in unrelated backend text. + */ export function isExpectedReticulumProxyErrorMessage(message: string): boolean { - const lower = message.toLowerCase(); - return ( - lower.includes('not running') || - message.includes('404') || - lower.includes('fetch failed') || + const lower = message.toLowerCase().trim(); + if (lower.includes('not running')) return true; + if (lower.includes('fetch failed')) return true; + if (lower.includes('rate limit exceeded')) return true; + // AbortSignal.timeout / fetch abort wording (word-bounded timeout; not bare digit runs) + if ( lower.includes('aborted') || - lower.includes('timeout') || - lower.includes('rate limit exceeded') - ); + lower.includes('timed out') || + lower.includes('due to timeout') || + /\btimeouts?\b/.test(lower) + ) { + return true; + } + // Sidecar manager: `sidecar GET|POST|PUT|DELETE failed: ` + if (httpStatusFromProxyMessage(lower) === 404) return true; + return false; +} + +/** + * Prefer structured fields (`status` / `statusCode`, AbortError / TimeoutError) + * when present; fall back to anchored message checks. + */ +export function isExpectedReticulumProxyError(err: unknown): boolean { + if (err == null) return false; + if (typeof err === 'object') { + const status = readNumericStatusField(err); + if (status === 404) return true; + const name = + 'name' in err && typeof (err as { name?: unknown }).name === 'string' + ? (err as { name: string }).name + : ''; + if (name === 'AbortError' || name === 'TimeoutError') return true; + } + const message = + err instanceof Error ? err.message : typeof err === 'string' ? err : errLikeMessage(err); + return isExpectedReticulumProxyErrorMessage(message); +} + +function errLikeMessage(err: unknown): string { + if (err != null && typeof err === 'object' && 'message' in err) { + const msg = (err as { message?: unknown }).message; + if (typeof msg === 'string') return msg; + } + return 'unknown error'; } export function isReticulumProxyIpcErrorEnvelope(