From dd5e07f9b4447f21919aea4688df757225935a21 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Thu, 20 Jun 2019 12:10:06 -0400 Subject: [PATCH] child_process: attach child in promisification This commit updates the custom exec() and execFile() promisification to attach the ChildProcess instance to the returned Promise. PR-URL: https://github.com/nodejs/node/pull/28325 Fixes: https://github.com/nodejs/node/issues/28244 Reviewed-By: Luigi Pinca Reviewed-By: Wyatt Preul --- doc/api/child_process.md | 16 ++++++----- lib/child_process.js | 27 ++++++++++++------- .../test-child-process-promisified.js | 20 +++++++++++--- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index b7c0d52a0fd5c8..3dc855f604bf76 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -214,10 +214,11 @@ Unlike the exec(3) POSIX system call, `child_process.exec()` does not replace the existing process and uses a shell to execute the command. If this method is invoked as its [`util.promisify()`][]ed version, it returns -a `Promise` for an `Object` with `stdout` and `stderr` properties. In case of an -error (including any error resulting in an exit code other than 0), a rejected -promise is returned, with the same `error` object given in the callback, but -with an additional two properties `stdout` and `stderr`. +a `Promise` for an `Object` with `stdout` and `stderr` properties. The returned +`ChildProcess` instance is attached to the `Promise` as a `child` property. In +case of an error (including any error resulting in an exit code other than 0), a +rejected promise is returned, with the same `error` object given in the +callback, but with an additional two properties `stdout` and `stderr`. ```js const util = require('util'); @@ -295,9 +296,10 @@ stderr output. If `encoding` is `'buffer'`, or an unrecognized character encoding, `Buffer` objects will be passed to the callback instead. If this method is invoked as its [`util.promisify()`][]ed version, it returns -a `Promise` for an `Object` with `stdout` and `stderr` properties. In case of an -error (including any error resulting in an exit code other than 0), a rejected -promise is returned, with the same `error` object given in the +a `Promise` for an `Object` with `stdout` and `stderr` properties. The returned +`ChildProcess` instance is attached to the `Promise` as a `child` property. In +case of an error (including any error resulting in an exit code other than 0), a +rejected promise is returned, with the same `error` object given in the callback, but with an additional two properties `stdout` and `stderr`. ```js diff --git a/lib/child_process.js b/lib/child_process.js index 66be7611dc9587..0d3785f0d5b9b8 100644 --- a/lib/child_process.js +++ b/lib/child_process.js @@ -149,17 +149,24 @@ function exec(command, options, callback) { const customPromiseExecFunction = (orig) => { return (...args) => { - return new Promise((resolve, reject) => { - orig(...args, (err, stdout, stderr) => { - if (err !== null) { - err.stdout = stdout; - err.stderr = stderr; - reject(err); - } else { - resolve({ stdout, stderr }); - } - }); + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; }); + + promise.child = orig(...args, (err, stdout, stderr) => { + if (err !== null) { + err.stdout = stdout; + err.stderr = stderr; + reject(err); + } else { + resolve({ stdout, stderr }); + } + }); + + return promise; }; }; diff --git a/test/parallel/test-child-process-promisified.js b/test/parallel/test-child-process-promisified.js index 877bf06662e4e0..bc623130994a2e 100644 --- a/test/parallel/test-child-process-promisified.js +++ b/test/parallel/test-child-process-promisified.js @@ -8,25 +8,37 @@ const exec = promisify(child_process.exec); const execFile = promisify(child_process.execFile); { - exec(`${process.execPath} -p 42`).then(common.mustCall((obj) => { + const promise = exec(`${process.execPath} -p 42`); + + assert(promise.child instanceof child_process.ChildProcess); + promise.then(common.mustCall((obj) => { assert.deepStrictEqual(obj, { stdout: '42\n', stderr: '' }); })); } { - execFile(process.execPath, ['-p', '42']).then(common.mustCall((obj) => { + const promise = execFile(process.execPath, ['-p', '42']); + + assert(promise.child instanceof child_process.ChildProcess); + promise.then(common.mustCall((obj) => { assert.deepStrictEqual(obj, { stdout: '42\n', stderr: '' }); })); } { - exec('doesntexist').catch(common.mustCall((err) => { + const promise = exec('doesntexist'); + + assert(promise.child instanceof child_process.ChildProcess); + promise.catch(common.mustCall((err) => { assert(err.message.includes('doesntexist')); })); } { - execFile('doesntexist', ['-p', '42']).catch(common.mustCall((err) => { + const promise = execFile('doesntexist', ['-p', '42']); + + assert(promise.child instanceof child_process.ChildProcess); + promise.catch(common.mustCall((err) => { assert(err.message.includes('doesntexist')); })); }