child_process: use signal.reason in child process abort

Fixes: #47814 PR-URL: #47817 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
nodejs · May 8, 2023 · 2dc4290 · 2dc4290
1 parent c15bafd
commit 2dc4290
Show file tree

Hide file tree

Showing 4 changed files with 162 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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();