From 4782113cebc990b54353e095db1eb6c5e654bdef Mon Sep 17 00:00:00 2001 From: Richie Foreman Date: Wed, 6 Aug 2025 19:31:42 -0400 Subject: [PATCH] =?UTF-8?q?fix(core):=20Improve=20errors=20in=20situations?= =?UTF-8?q?=20where=20the=20command=20spawn=20does=20=E2=80=A6=20(#5723)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../services/shellExecutionService.test.ts | 10 +++++ .../src/services/shellExecutionService.ts | 45 ++++++++++++++----- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index cfce08d2..2fe51a5e 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -185,6 +185,16 @@ describe('ShellExecutionService', () => { expect(result.error).toBe(spawnError); expect(result.exitCode).toBe(1); }); + + it('handles errors that do not fire the exit event', async () => { + const error = new Error('spawn abc ENOENT'); + const { result } = await simulateExecution('touch cat.jpg', (cp) => { + cp.emit('error', error); // No exit event is fired. + }); + + expect(result.error).toBe(error); + expect(result.exitCode).toBe(1); + }); }); describe('Aborting Commands', () => { diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index d1126a7d..3749fcf6 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -174,7 +174,19 @@ export class ShellExecutionService { child.stdout.on('data', (data) => handleOutput(data, 'stdout')); child.stderr.on('data', (data) => handleOutput(data, 'stderr')); child.on('error', (err) => { + const { stdout, stderr, finalBuffer } = cleanup(); error = err; + resolve({ + error, + stdout, + stderr, + rawOutput: finalBuffer, + output: stdout + (stderr ? `\n${stderr}` : ''), + exitCode: 1, + signal: null, + aborted: false, + pid: child.pid, + }); }); const abortHandler = async () => { @@ -200,18 +212,8 @@ export class ShellExecutionService { abortSignal.addEventListener('abort', abortHandler, { once: true }); - child.on('exit', (code, signal) => { - exited = true; - abortSignal.removeEventListener('abort', abortHandler); - - if (stdoutDecoder) { - stdout += stripAnsi(stdoutDecoder.decode()); - } - if (stderrDecoder) { - stderr += stripAnsi(stderrDecoder.decode()); - } - - const finalBuffer = Buffer.concat(outputChunks); + child.on('exit', (code: number, signal: NodeJS.Signals) => { + const { stdout, stderr, finalBuffer } = cleanup(); resolve({ rawOutput: finalBuffer, @@ -225,6 +227,25 @@ export class ShellExecutionService { pid: child.pid, }); }); + + /** + * Cleans up a process (and it's accompanying state) that is exiting or + * erroring and returns output formatted output buffers and strings + */ + function cleanup() { + exited = true; + abortSignal.removeEventListener('abort', abortHandler); + if (stdoutDecoder) { + stdout += stripAnsi(stdoutDecoder.decode()); + } + if (stderrDecoder) { + stderr += stripAnsi(stderrDecoder.decode()); + } + + const finalBuffer = Buffer.concat(outputChunks); + + return { stdout, stderr, finalBuffer }; + } }); return { pid: child.pid, result };