Skip to content

Commit

Permalink
child_process: use signal.reason in child process abort
Browse files Browse the repository at this point in the history
  • Loading branch information
debadree25 committed May 2, 2023
1 parent 32778b8 commit f2e3974
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 6 deletions.
7 changes: 3 additions & 4 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ const { Buffer } = require('buffer');
const { Pipe, constants: PipeConstants } = internalBinding('pipe_wrap');

const {
AbortError,
codes: errorCodes,
genericNodeError,
} = require('internal/errors');
Expand Down Expand Up @@ -712,12 +711,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', reason);
}
} catch (err) {
child.emit('error', err);
Expand Down Expand Up @@ -787,7 +786,7 @@ function spawn(file, args, options) {
}

function onAbortListener() {
abortChildProcess(child, killSignal);
abortChildProcess(child, killSignal, options.signal.reason);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ 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')
assert.rejects(promise, {
name: 'AbortError'
})
.then(common.mustCall());
ac.abort();
}
Expand All @@ -40,6 +42,6 @@ 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());
}
35 changes: 35 additions & 0 deletions test/parallel/test-child-process-fork-abort-signal.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,25 @@ 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, 'Error');
strictEqual(err.message, 'boom');
}));
process.nextTick(() => ac.abort(new Error('boom')));
}

{
// Test passing an already aborted signal to a forked child_process
const signal = AbortSignal.abort();
Expand All @@ -36,6 +55,22 @@ 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, 'Error');
strictEqual(err.message, 'boom');
}));
}

{
// Test passing a different kill signal
const signal = AbortSignal.abort();
Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-child-process-spawn-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,27 @@ 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, 'Error');
assert.strictEqual(e.message, 'custom abort error');
}));

controller.abort(new Error('custom abort error'));
}

{
// Verify that passing an already-aborted signal works.
const signal = AbortSignal.abort();
Expand All @@ -44,6 +65,24 @@ 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('custom abort error'));
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, 'Error');
assert.strictEqual(e.message, 'custom abort error');
}));
}

{
// Verify that waiting a bit and closing works
const controller = new AbortController();
Expand Down

0 comments on commit f2e3974

Please sign in to comment.