From 2dc4290662d57e378f9b8902b426940a5200be1a Mon Sep 17 00:00:00 2001 From: Debadree Chatterjee Date: Mon, 8 May 2023 13:47:03 +0530 Subject: child_process: use signal.reason in child process abort Fixes: https://github.com/nodejs/node/issues/47814 PR-URL: https://github.com/nodejs/node/pull/47817 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel Reviewed-By: Darshan Sen --- lib/child_process.js | 6 +- ...ild-process-exec-abortcontroller-promisified.js | 48 +++++++++++++- .../test-child-process-fork-abort-signal.js | 37 +++++++++++ .../test-child-process-spawn-controller.js | 77 ++++++++++++++++++++++ 4 files changed, 162 insertions(+), 6 deletions(-) diff --git a/lib/child_process.js b/lib/child_process.js index 5d3e2d63a7..59c37b9767 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -712,12 +712,12 @@ function normalizeSpawnArguments(file, args, options) { }; } -function abortChildProcess(child, killSignal) { +function abortChildProcess(child, killSignal, reason) { if (!child) return; try { if (child.kill(killSignal)) { - child.emit('error', new AbortError()); + child.emit('error', new AbortError(undefined, { cause: reason })); } } catch (err) { child.emit('error', err); @@ -787,7 +787,7 @@ function spawn(file, args, options) { } function onAbortListener() { - abortChildProcess(child, killSignal); + abortChildProcess(child, killSignal, options.signal.reason); } } diff --git a/test/parallel/test-child-process-exec-abortcontroller-promisified.js b/test/parallel/test-child-process-exec-abortcontroller-promisified.js index 0a409f8b99..d87b502ab8 100644 --- a/test/parallel/test-child-process-exec-abortcontroller-promisified.js +++ b/test/parallel/test-child-process-exec-abortcontroller-promisified.js @@ -18,11 +18,36 @@ const waitCommand = common.isWindows ? const ac = new AbortController(); const signal = ac.signal; const promise = execPromisifed(waitCommand, { signal }); - assert.rejects(promise, /AbortError/, 'post aborted sync signal failed') - .then(common.mustCall()); + assert.rejects(promise, { + name: 'AbortError', + cause: new DOMException('This operation was aborted', 'AbortError'), + }).then(common.mustCall()); ac.abort(); } +{ + const err = new Error('boom'); + const ac = new AbortController(); + const signal = ac.signal; + const promise = execPromisifed(waitCommand, { signal }); + assert.rejects(promise, { + name: 'AbortError', + cause: err + }).then(common.mustCall()); + ac.abort(err); +} + +{ + const ac = new AbortController(); + const signal = ac.signal; + const promise = execPromisifed(waitCommand, { signal }); + assert.rejects(promise, { + name: 'AbortError', + cause: 'boom' + }).then(common.mustCall()); + ac.abort('boom'); +} + { assert.throws(() => { execPromisifed(waitCommand, { signal: {} }); @@ -40,6 +65,23 @@ const waitCommand = common.isWindows ? const signal = AbortSignal.abort(); // Abort in advance const promise = execPromisifed(waitCommand, { signal }); - assert.rejects(promise, /AbortError/, 'pre aborted signal failed') + assert.rejects(promise, { name: 'AbortError' }) + .then(common.mustCall()); +} + +{ + const err = new Error('boom'); + const signal = AbortSignal.abort(err); // Abort in advance + const promise = execPromisifed(waitCommand, { signal }); + + assert.rejects(promise, { name: 'AbortError', cause: err }) + .then(common.mustCall()); +} + +{ + const signal = AbortSignal.abort('boom'); // Abort in advance + const promise = execPromisifed(waitCommand, { signal }); + + assert.rejects(promise, { name: 'AbortError', cause: 'boom' }) .then(common.mustCall()); } diff --git a/test/parallel/test-child-process-fork-abort-signal.js b/test/parallel/test-child-process-fork-abort-signal.js index 9982444c42..b963306fb1 100644 --- a/test/parallel/test-child-process-fork-abort-signal.js +++ b/test/parallel/test-child-process-fork-abort-signal.js @@ -21,6 +21,26 @@ const { fork } = require('child_process'); })); process.nextTick(() => ac.abort()); } + +{ + // Test aborting with custom error + const ac = new AbortController(); + const { signal } = ac; + const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), { + signal + }); + cp.on('exit', mustCall((code, killSignal) => { + strictEqual(code, null); + strictEqual(killSignal, 'SIGTERM'); + })); + cp.on('error', mustCall((err) => { + strictEqual(err.name, 'AbortError'); + strictEqual(err.cause.name, 'Error'); + strictEqual(err.cause.message, 'boom'); + })); + process.nextTick(() => ac.abort(new Error('boom'))); +} + { // Test passing an already aborted signal to a forked child_process const signal = AbortSignal.abort(); @@ -36,6 +56,23 @@ const { fork } = require('child_process'); })); } +{ + // Test passing an aborted signal with custom error to a forked child_process + const signal = AbortSignal.abort(new Error('boom')); + const cp = fork(fixtures.path('child-process-stay-alive-forever.js'), { + signal + }); + cp.on('exit', mustCall((code, killSignal) => { + strictEqual(code, null); + strictEqual(killSignal, 'SIGTERM'); + })); + cp.on('error', mustCall((err) => { + strictEqual(err.name, 'AbortError'); + strictEqual(err.cause.name, 'Error'); + strictEqual(err.cause.message, 'boom'); + })); +} + { // Test passing a different kill signal const signal = AbortSignal.abort(); diff --git a/test/parallel/test-child-process-spawn-controller.js b/test/parallel/test-child-process-spawn-controller.js index 5199c7a095..20facb09b3 100644 --- a/test/parallel/test-child-process-spawn-controller.js +++ b/test/parallel/test-child-process-spawn-controller.js @@ -27,6 +27,48 @@ const aliveScript = fixtures.path('child-process-stay-alive-forever.js'); controller.abort(); } +{ + // Verify that passing an AbortSignal with custom abort error works + const controller = new AbortController(); + const { signal } = controller; + const cp = spawn(process.execPath, [aliveScript], { + signal, + }); + + cp.on('exit', common.mustCall((code, killSignal) => { + assert.strictEqual(code, null); + assert.strictEqual(killSignal, 'SIGTERM'); + })); + + cp.on('error', common.mustCall((e) => { + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(e.cause.name, 'Error'); + assert.strictEqual(e.cause.message, 'boom'); + })); + + controller.abort(new Error('boom')); +} + +{ + const controller = new AbortController(); + const { signal } = controller; + const cp = spawn(process.execPath, [aliveScript], { + signal, + }); + + cp.on('exit', common.mustCall((code, killSignal) => { + assert.strictEqual(code, null); + assert.strictEqual(killSignal, 'SIGTERM'); + })); + + cp.on('error', common.mustCall((e) => { + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(e.cause, 'boom'); + })); + + controller.abort('boom'); +} + { // Verify that passing an already-aborted signal works. const signal = AbortSignal.abort(); @@ -44,6 +86,41 @@ const aliveScript = fixtures.path('child-process-stay-alive-forever.js'); })); } +{ + // Verify that passing an already-aborted signal with custom abort error + // works. + const signal = AbortSignal.abort(new Error('boom')); + const cp = spawn(process.execPath, [aliveScript], { + signal, + }); + cp.on('exit', common.mustCall((code, killSignal) => { + assert.strictEqual(code, null); + assert.strictEqual(killSignal, 'SIGTERM'); + })); + + cp.on('error', common.mustCall((e) => { + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(e.cause.name, 'Error'); + assert.strictEqual(e.cause.message, 'boom'); + })); +} + +{ + const signal = AbortSignal.abort('boom'); + const cp = spawn(process.execPath, [aliveScript], { + signal, + }); + cp.on('exit', common.mustCall((code, killSignal) => { + assert.strictEqual(code, null); + assert.strictEqual(killSignal, 'SIGTERM'); + })); + + cp.on('error', common.mustCall((e) => { + assert.strictEqual(e.name, 'AbortError'); + assert.strictEqual(e.cause, 'boom'); + })); +} + { // Verify that waiting a bit and closing works const controller = new AbortController(); -- cgit v1.2.1