diff --git a/src/test/test.ts b/src/test/test.ts index 2bdd90a..fff947a 100644 --- a/src/test/test.ts +++ b/src/test/test.ts @@ -255,3 +255,205 @@ describe('Fix: Asynchronous Function Execution', function () { assert.strictEqual(r2.result, false); }); }); + +// Fix: Worker Process Exit Handling (Issue #114) +// Ensures that when a worker process exits: +// 1. The deploy promise is not double-settled (deployReject after deployResolve) +// 2. All pending function invocations in the InvokeQueue are rejected so +// HTTP responses don't hang forever +describe('Fix: Worker Process Exit Handling (Issue #114)', function () { + // --- InvokeQueue.drain() --- + + it('should reject all pending invocations when drain() is called', done => { + // We create a minimal InvokeQueue-like structure to test the drain logic + // without importing the singleton (which shares state across tests) + const queue: Record< + string, + { resolve: (v: string) => void; reject: (r: string) => void } + > = {}; + + const push = (invoke: { + resolve: (v: string) => void; + reject: (r: string) => void; + }): string => { + const id = String(Math.random()); + queue[id] = invoke; + return id; + }; + + const drain = (reason: string): void => { + for (const [id, invoke] of Object.entries(queue)) { + invoke.reject(reason); + delete queue[id]; + } + }; + + let rejectedCount = 0; + const total = 3; + const reason = 'Worker exited unexpectedly'; + + for (let i = 0; i < total; i++) { + push({ + resolve: () => { + assert.fail('resolve should not be called during drain'); + }, + reject: (r: string) => { + assert.strictEqual(r, reason); + rejectedCount++; + if (rejectedCount === total) { + assert.strictEqual( + Object.keys(queue).length, + 0, + 'Queue should be empty after drain' + ); + done(); + } + } + }); + } + + drain(reason); + }); + + it('should not throw when drain() is called on an empty queue', () => { + const queue: Record< + string, + { resolve: (v: string) => void; reject: (r: string) => void } + > = {}; + + const drain = (reason: string): void => { + for (const [id, invoke] of Object.entries(queue)) { + invoke.reject(reason); + delete queue[id]; + } + }; + + // Should not throw + assert.doesNotThrow(() => drain('no-op')); + assert.strictEqual(Object.keys(queue).length, 0); + }); + + // --- InvokeQueue.has() --- + + it('should return true for a queued invocation and false after get()', () => { + const queue: Record< + string, + { resolve: (v: string) => void; reject: (r: string) => void } + > = {}; + + const push = (invoke: { + resolve: (v: string) => void; + reject: (r: string) => void; + }): string => { + const id = String(Math.random()); + queue[id] = invoke; + return id; + }; + + const has = (id: string): boolean => id in queue; + + const get = ( + id: string + ): { resolve: (v: string) => void; reject: (r: string) => void } => { + const invoke = queue[id]; + delete queue[id]; + return invoke; + }; + + const id = push({ + resolve: () => { + /* noop */ + }, + reject: () => { + /* noop */ + } + }); + + assert.strictEqual(has(id), true, 'has() should return true after push'); + assert.strictEqual( + has('nonexistent'), + false, + 'has() should return false for unknown id' + ); + + const invoke = get(id); + assert.ok(invoke, 'get() should return the invocation'); + assert.strictEqual( + has(id), + false, + 'has() should return false after get()' + ); + }); + + // --- Settled guard pattern --- + + it('should only call resolve once even when reject is also attempted (deploy success then exit)', () => { + let settled = false; + let resolveCount = 0; + let rejectCount = 0; + + const safeResolve = (): void => { + if (!settled) { + settled = true; + resolveCount++; + } + }; + + const safeReject = (_err: Error): void => { + if (!settled) { + settled = true; + rejectCount++; + } + }; + + // Simulate successful deploy then worker exit + safeResolve(); // deploy succeeds + safeReject(new Error('exit')); // worker exits after — should be no-op + + assert.strictEqual( + resolveCount, + 1, + 'resolve should be called exactly once' + ); + assert.strictEqual( + rejectCount, + 0, + 'reject should not be called after resolve' + ); + }); + + it('should call reject once when worker exits before deploy completes', () => { + let settled = false; + let resolveCount = 0; + let rejectCount = 0; + + const safeResolve = (): void => { + if (!settled) { + settled = true; + resolveCount++; + } + }; + + const safeReject = (_err: Error): void => { + if (!settled) { + settled = true; + rejectCount++; + } + }; + + // Simulate worker exit before deploy completes + safeReject(new Error('exit before deploy')); + safeResolve(); // should be no-op + + assert.strictEqual( + rejectCount, + 1, + 'reject should be called exactly once' + ); + assert.strictEqual( + resolveCount, + 0, + 'resolve should not be called after reject' + ); + }); +}); diff --git a/src/utils/deploy.ts b/src/utils/deploy.ts index d1870f0..e86e729 100644 --- a/src/utils/deploy.ts +++ b/src/utils/deploy.ts @@ -47,14 +47,29 @@ export const deployProcess = async ( // Wait for load result let deployResolve: (value: void) => void; let deployReject: (reason: Error) => void; + let settled = false; const promise = new Promise((resolve, reject) => { deployResolve = resolve; deployReject = reject; }); + const safeResolve = (): void => { + if (!settled) { + settled = true; + deployResolve(); + } + }; + + const safeReject = (err: Error): void => { + if (!settled) { + settled = true; + deployReject(err); + } + }; + proc.on('error', (err: Error) => { - deployReject(err); + safeReject(err); }); proc.on('message', (payload: WorkerMessageUnknown) => { @@ -66,7 +81,7 @@ export const deployProcess = async ( application.proc = proc; application.deployment = deployment; - deployResolve(); + safeResolve(); break; } @@ -78,6 +93,7 @@ export const deployProcess = async ( // Get the invocation id in order to retrieve the callbacks // for resolving the call, this deletes the invocation object + if (!invokeQueue.has(invokeResult.id)) break; const invoke = invokeQueue.get(invokeResult.id); invoke.resolve(JSON.stringify(invokeResult.result)); break; @@ -91,7 +107,7 @@ export const deployProcess = async ( proc.on('exit', code => { // The application may have been ended unexpectedly, // probably segmentation fault (exit code 139 in Linux) - deployReject( + safeReject( new Error( `Deployment '${resource.id}' process exited with code: ${ code || 'unknown' @@ -99,10 +115,10 @@ export const deployProcess = async ( ) ); - // TODO: How to implement the exit properly? We cannot reject easily - // the promise from the call if the process exits during the call. - // Also if exits during the call it will try to call deployReject - // which is completely out of scope and the promise was fullfilled already + // Drain all pending invocations so their HTTP responses don't hang + invokeQueue.drain( + `Worker process for '${resource.id}' exited unexpectedly (code: ${code || 'unknown'})` + ); }); return promise; diff --git a/src/utils/invoke.ts b/src/utils/invoke.ts index c091a60..18951a8 100644 --- a/src/utils/invoke.ts +++ b/src/utils/invoke.ts @@ -19,6 +19,17 @@ class InvokeQueue { delete this.queue[id]; return invoke; } + + public has(id: string): boolean { + return id in this.queue; + } + + public drain(reason: string): void { + for (const [id, invoke] of Object.entries(this.queue)) { + invoke.reject(reason); + delete this.queue[id]; + } + } } export const invokeQueue = new InvokeQueue();