From c6ccacab322e14974968efe8d4962e0859b03d50 Mon Sep 17 00:00:00 2001 From: Alexandr Date: Tue, 28 Jul 2026 17:42:31 +0200 Subject: [PATCH 1/2] test(webactor): gate worker supervisor tests on conditions, not fixed sleeps Three tests raced the worker startup they were meant to observe. The handshake takes 49-66ms on an idle machine, so a fixed 100ms sleep left 35-50ms of slack; a loaded CI runner running 16 test files in parallel blows through that and the assertion sees nothing yet. Wait for the awaited condition instead. Liveness assertions move into vi.waitFor; the "no second restart happened" check keeps a fixed settle window, which is safe because it fails open on a slow machine. The manual-termination test had a second defect that no timeout tolerance could fix: it terminated the worker on a blind 300ms timer while its own wait was also 300ms, so on a slow machine it killed the worker before the handshake completed and the supervisor never restarted it. Terminate after a ping/pong proves the worker is actually up, so the test exercises what it claims: a live worker dies and gets replaced. Co-Authored-By: Claude Opus 5 (1M context) --- .../webactor/tests/worker/supervisor.test.ts | 65 ++++++++++++------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/packages/webactor/tests/worker/supervisor.test.ts b/packages/webactor/tests/worker/supervisor.test.ts index 686244e..625595a 100644 --- a/packages/webactor/tests/worker/supervisor.test.ts +++ b/packages/webactor/tests/worker/supervisor.test.ts @@ -1,9 +1,9 @@ import '../locks'; import { Worker } from '@apacheli/web-workers'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; -import { Actor } from '../../src/types'; +import { Actor, AnyData } from '../../src/types'; import { applyWorkerSupervisor } from '../../src/worker/applyWorkerSupervisor'; function createWorker() { @@ -18,6 +18,10 @@ function createErrorWorker() { }); } +function isTaggedData(data: AnyData, type: string) { + return typeof data === 'object' && data !== null && 'type' in data && data.type === type; +} + describe('Worker Supervisor Tests with Real Workers', () => { let supervisedActor: Actor; let workers: Worker[] = []; @@ -123,7 +127,8 @@ describe('Worker Supervisor Tests with Real Workers', () => { }); supervisedActor.launch(); - await new Promise((resolve) => setTimeout(resolve, 100)); + await vi.waitFor(() => expect(retryCount).toBeGreaterThanOrEqual(1), { timeout: 5000, interval: 10 }); + await new Promise((resolve) => setTimeout(resolve, 200)); expect(retryCount).toBe(1); expect(createCount).toBe(1); @@ -223,15 +228,16 @@ describe('Worker Supervisor Tests with Real Workers', () => { supervisedActor.launch(); - // Wait for error worker to fail and restart cycles - await new Promise((resolve) => setTimeout(resolve, 100)); + await vi.waitFor( + () => { + expect(createCount).toBeGreaterThan(1); + expect(restartReasons.length).toBeGreaterThan(0); + expect(workers.length).toBeGreaterThan(1); + }, + { timeout: 5000, interval: 10 }, + ); console.log(`Final state: createCount=${createCount}, restartReasons:`, restartReasons); - - // Should have created multiple workers due to errors - expect(createCount).toBeGreaterThan(1); - expect(restartReasons.length).toBeGreaterThan(0); - expect(workers.length).toBeGreaterThan(1); }); it('should restart worker when terminated manually', async () => { @@ -243,15 +249,6 @@ describe('Worker Supervisor Tests with Real Workers', () => { console.log(`Creating terminate-test worker #${createCount}`); const worker = createWorker(); workers.push(worker); - - // Terminate the first worker after short delay - if (createCount === 1) { - setTimeout(() => { - console.log('Manually terminating first worker...'); - worker.terminate(); - }, 300); - } - return worker; }; @@ -267,16 +264,34 @@ describe('Worker Supervisor Tests with Real Workers', () => { }, }); + let pongs = 0; + supervisedActor.addEventListener('message', (envelope) => { + if (isTaggedData(envelope.data, 'pong')) pongs++; + }); + supervisedActor.launch(); - // Wait for termination and restart - await new Promise((resolve) => setTimeout(resolve, 300)); + // terminating before the worker has connected leaves nothing for the supervisor to notice + await vi.waitFor( + () => { + supervisedActor.postMessage({ type: 'ping' }); + expect(pongs).toBeGreaterThan(0); + }, + { timeout: 5000, interval: 20 }, + ); - console.log(`Termination test result: createCount=${createCount}, restartReasons:`, restartReasons); + console.log('Manually terminating first worker...'); + workers[0].terminate(); + + await vi.waitFor( + () => { + expect(createCount).toBeGreaterThanOrEqual(2); + expect(workers.length).toBeGreaterThanOrEqual(2); + }, + { timeout: 5000, interval: 10 }, + ); - // Should have restarted after termination - expect(createCount).toBeGreaterThanOrEqual(2); - expect(workers.length).toBeGreaterThanOrEqual(2); + console.log(`Termination test result: createCount=${createCount}, restartReasons:`, restartReasons); }); it('should handle worker that throws error on message', async () => { From fcd0eed8d1ea060abd9e599e94e584428455b5c5 Mon Sep 17 00:00:00 2001 From: Alexandr Date: Tue, 28 Jul 2026 18:46:34 +0200 Subject: [PATCH 2/2] fix(webactor): notice a worker that dies before its handshake answers The liveness watch keys off a lock name carried in the handshake reply, so it cannot be armed until that reply arrives. Until then only the worker's error event could report trouble, and a worker that died silently in that window was never noticed: the handshake request retried forever and the supervisor kept a dead worker indefinitely. Measured: a worker terminated 15ms after spawn, one calling self.close() at startup, and one that stays up but never answers all produced zero restart decisions. getAbortSignal lets the caller bound that wait with the same primitive the rest of the library takes, rather than a bespoke timeout number. It is a factory because a supervisor relaunches, and one signal would already be spent by the second worker. All three cases above now reach shouldRetry, carrying whatever the signal aborted with, so AbortSignal.timeout surfaces as a TimeoutError. The catch on the handshake was swallowing the fix: it mapped aborts to a sentinel and rethrew everything else, so a failure surfaced as an unhandled rejection rather than a restart decision. It now distinguishes by source instead of by reason shape - only the supervisor's own teardown stays quiet, so a plain AbortController from the caller counts as a failed handshake rather than being mistaken for that teardown. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/lucky-otters-listen.md | 13 ++ .../src/worker/applyWorkerSupervisor.ts | 15 +- .../webactor/tests/worker/supervisor.test.ts | 141 ++++++++++++++++++ 3 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 .changeset/lucky-otters-listen.md diff --git a/.changeset/lucky-otters-listen.md b/.changeset/lucky-otters-listen.md new file mode 100644 index 0000000..0e8efcc --- /dev/null +++ b/.changeset/lucky-otters-listen.md @@ -0,0 +1,13 @@ +--- +'webactor': minor +--- + +Notice a worker that dies before its handshake answers. + +A worker supervisor watches its worker's liveness through a lock whose key it learns from the handshake reply, so the watch can only be armed once that reply arrives. Before it did, the only detector was the worker's own `error` event. A worker that died quietly in that window — killed by the host, or closing itself — left the supervisor holding a dead worker forever: no restart, no error, nothing. A worker that came up but never answered was equally invisible. + +`applyWorkerSupervisor` now accepts `getAbortSignal`, a factory consulted once per launch, so the handshake can be bounded the same way every other operation in the library is: `getAbortSignal: () => AbortSignal.timeout(2000)`. It is a factory rather than a plain signal because a supervisor relaunches, and one signal would already be spent by the second worker. Whatever the returned signal aborts with reaches `shouldRetry` as the reason, so an `AbortSignal.timeout` arrives as a `TimeoutError`. + +Nothing changes when it is omitted. A deadline tight enough to be useful would misfire on a loaded machine, where a handshake legitimately takes hundreds of milliseconds, so the choice stays with the caller. + +A handshake that fails for any reason other than the supervisor's own teardown now reaches the restart decision too. It previously became an unhandled rejection instead, which meant an undeliverable handshake was reported to nobody and restarted nothing. diff --git a/packages/webactor/src/worker/applyWorkerSupervisor.ts b/packages/webactor/src/worker/applyWorkerSupervisor.ts index 6c06e37..38870d5 100644 --- a/packages/webactor/src/worker/applyWorkerSupervisor.ts +++ b/packages/webactor/src/worker/applyWorkerSupervisor.ts @@ -21,8 +21,10 @@ export function applyWorkerSupervisor( WorkerConstructor: () => Worker, { shouldRetry, + getAbortSignal, }: { shouldRetry: (reason?: unknown | Reason | Error | ErrorEvent) => boolean | Promise; + getAbortSignal?: () => undefined | AbortSignal; }, ): Actor { const proxy = createEnvelopeChannel(); @@ -52,13 +54,22 @@ export function applyWorkerSupervisor( .catch(catchAbortToSymbol); }; - request(messagePort, THREAD_ID_REQUEST, { abortSignal: abortController.signal }) + const callerSignal = getAbortSignal?.(); + const handshakeSignal = + callerSignal === undefined + ? abortController.signal + : AbortSignal.any([abortController.signal, callerSignal]); + + request(messagePort, THREAD_ID_REQUEST, { abortSignal: handshakeSignal }) .then((envelope) => { if (isObject(envelope.data) && isStringField(envelope.data, 'threadId')) { onUnlockThreadId(envelope.data.threadId); } }) - .catch(catchAbortToSymbol); + .catch((error) => { + if (abortController.signal.aborted) return; + decide(error); + }); const errorOff = on(worker, 'error', (error) => decide(error)); diff --git a/packages/webactor/tests/worker/supervisor.test.ts b/packages/webactor/tests/worker/supervisor.test.ts index 625595a..c92fe96 100644 --- a/packages/webactor/tests/worker/supervisor.test.ts +++ b/packages/webactor/tests/worker/supervisor.test.ts @@ -294,6 +294,147 @@ describe('Worker Supervisor Tests with Real Workers', () => { console.log(`Termination test result: createCount=${createCount}, restartReasons:`, restartReasons); }); + it('should detect a worker that dies before the handshake when given an abort signal', async () => { + let createCount = 0; + const reasons: unknown[] = []; + + const workerConstructor = () => { + createCount++; + const worker = createWorker(); + workers.push(worker); + if (createCount === 1) worker.terminate(); + return worker; + }; + + supervisedActor = applyWorkerSupervisor(workerConstructor, { + getAbortSignal: () => AbortSignal.timeout(150), + shouldRetry: async (reason) => { + reasons.push(reason); + return false; + }, + }); + + supervisedActor.launch(); + + await vi.waitFor(() => expect(reasons.length).toBeGreaterThan(0), { timeout: 5000, interval: 10 }); + + expect(reasons[0]).toBeInstanceOf(Error); + expect(String(reasons[0])).toContain('TimeoutError'); + expect(createCount).toBe(1); + }); + + it('should leave a worker that dies before the handshake unnoticed without an abort signal', async () => { + let createCount = 0; + let decisions = 0; + + const workerConstructor = () => { + createCount++; + const worker = createWorker(); + workers.push(worker); + if (createCount === 1) worker.terminate(); + return worker; + }; + + supervisedActor = applyWorkerSupervisor(workerConstructor, { + shouldRetry: async () => { + decisions++; + return false; + }, + }); + + supervisedActor.launch(); + await new Promise((resolve) => setTimeout(resolve, 600)); + + expect(decisions).toBe(0); + expect(createCount).toBe(1); + }); + + it('should build a fresh abort signal for every relaunch', async () => { + let createCount = 0; + let signalCount = 0; + const reasons: unknown[] = []; + + const workerConstructor = () => { + createCount++; + const worker = createWorker(); + workers.push(worker); + if (createCount <= 2) worker.terminate(); + return worker; + }; + + supervisedActor = applyWorkerSupervisor(workerConstructor, { + getAbortSignal: () => { + signalCount++; + return AbortSignal.timeout(150); + }, + shouldRetry: async (reason) => { + reasons.push(reason); + return reasons.length < 2; + }, + }); + + supervisedActor.launch(); + + // a single signal would already be spent here, so the second worker would go unwatched + await vi.waitFor(() => expect(reasons.length).toBe(2), { timeout: 5000, interval: 10 }); + + expect(signalCount).toBe(createCount); + expect(createCount).toBe(2); + }); + + it('should treat a plain abort from the caller as a failed handshake', async () => { + let createCount = 0; + const reasons: unknown[] = []; + const abortController = new AbortController(); + + const workerConstructor = () => { + createCount++; + const worker = createWorker(); + workers.push(worker); + if (createCount === 1) worker.terminate(); + return worker; + }; + + supervisedActor = applyWorkerSupervisor(workerConstructor, { + getAbortSignal: () => abortController.signal, + shouldRetry: async (reason) => { + reasons.push(reason); + return false; + }, + }); + + supervisedActor.launch(); + abortController.abort(); + + await vi.waitFor(() => expect(reasons.length).toBeGreaterThan(0), { timeout: 5000, interval: 10 }); + + expect(createCount).toBe(1); + }); + + it('should not ask for a restart decision when the supervisor itself is closed', async () => { + let decisions = 0; + + const workerConstructor = () => { + const worker = createWorker(); + workers.push(worker); + return worker; + }; + + supervisedActor = applyWorkerSupervisor(workerConstructor, { + getAbortSignal: () => AbortSignal.timeout(5000), + shouldRetry: async () => { + decisions++; + return false; + }, + }); + + supervisedActor.launch(); + supervisedActor.close(); + await new Promise((resolve) => setTimeout(resolve, 300)); + + expect(decisions).toBe(0); + }); + it('should handle worker that throws error on message', async () => { let createCount = 0; let messageErrorCount = 0;