diff --git a/src/main.ts b/src/main.ts index 00c521e..00bab99 100644 --- a/src/main.ts +++ b/src/main.ts @@ -41,6 +41,13 @@ if (started) { app.quit(); } +// Safety net — observability only, never recovery: rejections that escape the +// local catches are logged, not swallowed or recovered. Local `.catch` +// handling remains the real mechanism (hardening plan, Fase 4). +process.on('unhandledRejection', (reason) => { + console.error('[dw] unhandledRejection:', reason); +}); + let ptys: PtyManager | null = null; let broker: Broker | null = null; diff --git a/src/main/broker.test.ts b/src/main/broker.test.ts index 32af4b1..3dd43ae 100644 --- a/src/main/broker.test.ts +++ b/src/main/broker.test.ts @@ -303,6 +303,7 @@ describe('Broker (unit, stub deps)', () => { const injected: Array<{ to: string; body: string }> = []; const awaited: Array<{ to: string; timeoutMs: number }> = []; const slowTargets = new Set(); + const failingTargets = new Set(); const rolloverTargets = new Set(); const spawned: Array> = []; const killed: string[] = []; @@ -318,6 +319,9 @@ describe('Broker (unit, stub deps)', () => { // isolates just the new output — exactly like the real PTY mirror. awaitQuiet: async (id: string, timeoutMs: number) => { awaited.push({ to: id, timeoutMs }); + // A peer whose quiet-wait explodes: the exchange throws mid-ask, so + // handleAsk's Promise.all rejects — the dispatch catch must answer. + if (failingTargets.has(id)) throw new Error('exchange exploded'); if (slowTargets.has(id)) await new Promise((r) => setTimeout(r, 400)); const answer = answers.get(id); if (answer) captured.set(id, (captured.get(id) ?? '') + answer); @@ -387,6 +391,7 @@ describe('Broker (unit, stub deps)', () => { answers.clear(); captured.clear(); slowTargets.clear(); + failingTargets.clear(); rolloverTargets.clear(); injected.length = 0; awaited.length = 0; @@ -503,6 +508,16 @@ describe('Broker (unit, stub deps)', () => { expect(res).toEqual({ ok: false, error: 'ask needs a terminal target' }); }); + it('answers the socket when an ask explodes mid-exchange', async () => { + // The exchange throws for this peer, so handleAsk's internal Promise.all + // rejects; the dispatch catch must answer the socket instead of leaking + // an unhandled rejection. + addTerminal('peer1', 'Peer One'); + failingTargets.add('peer1'); + const res = await rpc(sock, { cmd: 'ask', from: 'lead', target: 'Peer One', body: 'hi' }); + expect(res).toEqual({ ok: false, error: 'exchange exploded' }); + }); + it('broadcasts to explicit targets with a broadcast id', async () => { addTerminal('peer1', 'Peer One'); addTerminal('peer2', 'Peer Two'); @@ -642,6 +657,15 @@ describe('Broker (unit, stub deps)', () => { expect(res).toEqual({ ok: false, error: 'boom' }); }); + it('answers the socket when portal op=new throws outside the internal try', async () => { + // The `new` branch runs before handlePortal's internal try; a throw there + // must be answered by the dispatch catch, not leaked as an unhandled + // rejection. + portals.create.mockImplementationOnce(() => { throw new Error('portal factory exploded'); }); + const res = await rpc(sock, { cmd: 'portal', from: 'lead', op: 'new', target: '', arg: 'https://x' }); + expect(res).toEqual({ ok: false, error: 'portal factory exploded' }); + }); + it('portal read ops return their payloads', async () => { addPortal('p1', 'Portal One'); const js = await rpc(sock, { cmd: 'portal', from: 'lead', op: 'js', target: 'p1', arg: '1+1' }); diff --git a/src/main/broker.ts b/src/main/broker.ts index c92d033..8ffc4a5 100644 --- a/src/main/broker.ts +++ b/src/main/broker.ts @@ -108,7 +108,11 @@ export class Broker { } switch (req.cmd) { case 'ask': - void this.handleAsk(socket, req); + // The internal Promise.all can reject (e.g. a PTY dies mid-ask); + // never leak that as an unhandled rejection — answer the terminal. + void this.handleAsk(socket, req).catch((err: unknown) => { + this.respond(socket, { ok: false, error: (err as Error).message }); + }); return; case 'check': return this.handleCheck(socket, req.from, req.target); @@ -127,7 +131,11 @@ export class Broker { req.chain, ); case 'portal': - void this.handlePortal(socket, req); + // Covers the residue outside handlePortal's internal try (op=new and + // the pre-try wiring); the internal try answers for its own ops. + void this.handlePortal(socket, req).catch((err: unknown) => { + this.respond(socket, { ok: false, error: (err as Error).message }); + }); return; case 'contract': return this.handleContract(socket, req); diff --git a/src/main/ptyManager.test.ts b/src/main/ptyManager.test.ts index 25d2cb7..2c4a621 100644 --- a/src/main/ptyManager.test.ts +++ b/src/main/ptyManager.test.ts @@ -1,9 +1,10 @@ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { GraphStore } from './graphStore'; import { PtyManager } from './ptyManager'; +import * as processTree from './processTree'; const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); @@ -101,3 +102,56 @@ describe('PtyManager spawn-time role ordering', () => { expect(text).toContain('Dogwalker role was updated'); }, 10_000); }); + +// ── safety net: the memory poller must never become an unhandled rejection +// source. listProcesses() normally resolves [] on error, but a rejection +// anywhere in checkMemory (e.g. a throwing process-tree implementation) must +// be logged by the interval's defensive catch — never leak. +describe('PtyManager memory poller safety net', () => { + let dir = ''; + let graph: GraphStore; + let ptys: PtyManager; + let term = ''; + + const webContents = { + send: () => {}, + isDestroyed: () => false, + } as unknown as ConstructorParameters[0]; + + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dw-pty-mem-')); + graph = new GraphStore(); + ptys = new PtyManager(webContents, graph, { socketPath: dir, shimDir: dir }); + term = ptys.spawn({ + preset: 'shell', name: 'mem-term', stableId: 'mem-term', + cols: 80, rows: 24, workspaceId: 'mem', floorName: 'ground', cwd: dir, + }).id; + }); + + afterAll(async () => { + ptys.killAll(); + await wait(200); + try { + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + /* best-effort cleanup */ + } + }); + + it('logs instead of leaking a rejection when a memory poll fails', async () => { + ptys.setMemoryLimit(term, 256); // starts the 5s poller + const procSpy = vi.spyOn(processTree, 'listProcesses').mockRejectedValue(new Error('ps boom')); + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + await wait(6_500); // one real poll cycle (5s interval) + expect(errSpy).toHaveBeenCalledWith( + expect.stringContaining('[dw] memory check failed'), + expect.any(Error), + ); + } finally { + procSpy.mockRestore(); + errSpy.mockRestore(); + ptys.setMemoryLimit(term, 0); // stops the poller + } + }, 30_000); +}); diff --git a/src/main/ptyManager.ts b/src/main/ptyManager.ts index 2358c7e..43f3439 100644 --- a/src/main/ptyManager.ts +++ b/src/main/ptyManager.ts @@ -388,7 +388,14 @@ export class PtyManager { private syncMemoryPoller(): void { const wanted = [...this.entries.values()].some((e) => e.memoryLimitMB > 0); if (wanted && !this.memoryTimer) { - this.memoryTimer = setInterval(() => void this.checkMemory(), MEMORY_POLL_MS); + this.memoryTimer = setInterval(() => { + void this.checkMemory().catch((err: unknown) => { + // Defensive: the poller must never become an unhandled rejection + // source. Observability only — the limit enforcement stays in + // checkMemory's own paths. + console.error('[dw] memory check failed:', err); + }); + }, MEMORY_POLL_MS); } else if (!wanted && this.memoryTimer) { clearInterval(this.memoryTimer); this.memoryTimer = null; diff --git a/src/main/routineService.test.ts b/src/main/routineService.test.ts index 5850949..5b55e7a 100644 --- a/src/main/routineService.test.ts +++ b/src/main/routineService.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -65,4 +65,27 @@ describe('RoutineService (integration, real PTY)', () => { expect(routines.list('rt').find((r) => r.id === routine.id)?.status).toBe('paused'); routines.remove(routine.id); }, 30_000); + + it('logs instead of leaking a rejection when a tick throws before its internal try', async () => { + // onUpdate fires before tick's internal try; a throw there rejects the + // tick. The interval's defensive catch must log it, never surface an + // unhandled rejection (which vitest would fail the suite on). + const boomTerm = ptys.spawn({ preset: 'shell', cols: 80, rows: 24, workspaceId: 'rt', stableId: 'boom-term', cwd: '', name: 'boom-term' }).id; + const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const throwing = new RoutineService(dir, ptys, () => { throw new Error('onUpdate boom'); }); + try { + const routine = throwing.create('rt', { + name: 'boom', targetStableId: 'boom-term', prompt: 'echo X', intervalMs: 5_000, + }); + await wait(6_500); // one real interval cycle (5s min) + expect(errSpy).toHaveBeenCalledWith( + expect.stringContaining(`[dw] routine ${routine.id} tick failed`), + expect.any(Error), + ); + } finally { + throwing.disposeAll(); + ptys.kill(boomTerm); + errSpy.mockRestore(); + } + }, 30_000); }); diff --git a/src/main/routineService.ts b/src/main/routineService.ts index 5b34815..63f7763 100644 --- a/src/main/routineService.ts +++ b/src/main/routineService.ts @@ -104,7 +104,14 @@ export class RoutineService { this.disarm(r.id); this.timers.set( r.id, - setInterval(() => void this.tick(r.id), r.intervalMs), + setInterval(() => { + void this.tick(r.id).catch((err: unknown) => { + // Defensive: tick's internal try/finally covers the run itself; + // this only catches pre-try throws so the scheduler never leaks an + // unhandled rejection every interval. Observability only. + console.error(`[dw] routine ${r.id} tick failed:`, err); + }); + }, r.intervalMs), ); } diff --git a/src/renderer.tsx b/src/renderer.tsx index 821bd40..f394c0b 100644 --- a/src/renderer.tsx +++ b/src/renderer.tsx @@ -4,6 +4,13 @@ import '@xterm/xterm/css/xterm.css'; import './index.css'; import { App } from './app/App'; +// Safety net — observability only, never recovery: anything that still escapes +// the local catches lands here as a structured log (mirrored to the main +// process stdout). It does not replace per-call `.catch` handling. +window.addEventListener('unhandledrejection', (event) => { + console.error('[dw] unhandledrejection:', event.reason); +}); + const container = document.getElementById('root'); if (!container) throw new Error('missing #root'); createRoot(container).render();