child_process: setup stdio on error when possible

cjihrig · BridgeAR · commit 2bc177aa4fa6 · 2019-05-21T13:13:24.000+02:00
As more spawn() errors are classified as runtime errors, it's no longer appropriate to only check UV_ENOENT when determining if stdio can be setup. This commit reverses the check to look for EMFILE and ENFILE specifically. PR-URL: #27696 Fixes: #26852 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -375,12 +375,11 @@ ChildProcess.prototype.spawn = function(options) {
       err === UV_ENFILE ||
       err === UV_ENOENT) {
     process.nextTick(onErrorNT, this, err);
+
     // There is no point in continuing when we've hit EMFILE or ENFILE
     // because we won't be able to set up the stdio file descriptors.
-    // It's kind of silly that the de facto spec for ENOENT (the test suite)
-    // mandates that stdio _is_ set up, even if there is no process on the
-    // receiving end, but it is what it is.
-    if (err !== UV_ENOENT) return err;
+    if (err === UV_EMFILE || err === UV_ENFILE)
+      return err;
   } else if (err) {
     // Close all opened fds on error
     for (i = 0; i < stdio.length; i++) {
diff --git a/test/parallel/test-child-process-spawn-error.js b/test/parallel/test-child-process-spawn-error.js
@@ -30,6 +30,16 @@ const spawnargs = ['bar'];
 assert.strictEqual(fs.existsSync(enoentPath), false);
 
 const enoentChild = spawn(enoentPath, spawnargs);
+
+// Verify that stdio is setup if the error is not EMFILE or ENFILE.
+assert.notStrictEqual(enoentChild.stdin, undefined);
+assert.notStrictEqual(enoentChild.stdout, undefined);
+assert.notStrictEqual(enoentChild.stderr, undefined);
+assert(Array.isArray(enoentChild.stdio));
+assert.strictEqual(enoentChild.stdio[0], enoentChild.stdin);
+assert.strictEqual(enoentChild.stdio[1], enoentChild.stdout);
+assert.strictEqual(enoentChild.stdio[2], enoentChild.stderr);
+
 enoentChild.on('error', common.mustCall(function(err) {
   assert.strictEqual(err.code, 'ENOENT');
   assert.strictEqual(err.errno, 'ENOENT');
diff --git a/test/sequential/test-child-process-emfile.js b/test/sequential/test-child-process-emfile.js
@@ -58,6 +58,12 @@ for (;;) {
 // Should emit an error, not throw.
 const proc = child_process.spawn(process.execPath, ['-e', '0']);
 
+// Verify that stdio is not setup on EMFILE or ENFILE.
+assert.strictEqual(proc.stdin, undefined);
+assert.strictEqual(proc.stdout, undefined);
+assert.strictEqual(proc.stderr, undefined);
+assert.strictEqual(proc.stdio, undefined);
+
 proc.on('error', common.mustCall(function(err) {
   assert.strictEqual(err.code, 'EMFILE');
 }));
