process: provide dummy stdio for non-console Windows apps

addaleax · BridgeAR · commit 8ff8af5de932 · 2018-12-05T20:07:16.000+01:00
The only known condition where we could not provide appropriate stdio streams so far were non-console Windows applications. Since this issue has come up a few times in our issue tracker now, switch to providing dummy streams for these cases instead. If there are other valid cases in which `uv_guess_handle` fails, and where there is a more sensible way to provide stdio, we’ll probably still find out because the streams don’t work properly either way. Refs: nodejs/help#1251 PR-URL: #20640 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -1778,20 +1778,6 @@ An attempt was made to load a module with an unknown or unsupported format.
 An invalid or unknown process signal was passed to an API expecting a valid
 signal (such as [`subprocess.kill()`][]).
 
-<a id="ERR_UNKNOWN_STDIN_TYPE"></a>
-### ERR_UNKNOWN_STDIN_TYPE
-
-An attempt was made to launch a Node.js process with an unknown `stdin` file
-type. This error is usually an indication of a bug within Node.js itself,
-although it is possible for user code to trigger it.
-
-<a id="ERR_UNKNOWN_STREAM_TYPE"></a>
-### ERR_UNKNOWN_STREAM_TYPE
-
-An attempt was made to launch a Node.js process with an unknown `stdout` or
-`stderr` file type. This error is usually an indication of a bug within Node.js
-itself, although it is possible for user code to trigger it.
-
 <a id="ERR_V8BREAKITERATOR"></a>
 ### ERR_V8BREAKITERATOR
 
@@ -2048,6 +2034,28 @@ kind of internal Node.js error that should not typically be triggered by user
 code. Instances of this error point to an internal bug within the Node.js
 binary itself.
 
+<a id="ERR_UNKNOWN_STDIN_TYPE"></a>
+### ERR_UNKNOWN_STDIN_TYPE
+<!-- YAML
+added: v8.0.0
+removed: REPLACEME
+-->
+
+An attempt was made to launch a Node.js process with an unknown `stdin` file
+type. This error is usually an indication of a bug within Node.js itself,
+although it is possible for user code to trigger it.
+
+<a id="ERR_UNKNOWN_STREAM_TYPE"></a>
+### ERR_UNKNOWN_STREAM_TYPE
+<!-- YAML
+added: v8.0.0
+removed: REPLACEME
+-->
+
+An attempt was made to launch a Node.js process with an unknown `stdout` or
+`stderr` file type. This error is usually an indication of a bug within Node.js
+itself, although it is possible for user code to trigger it.
+
 <a id="ERR_VALUE_OUT_OF_RANGE"></a>
 ### ERR_VALUE_OUT_OF_RANGE
 <!-- YAML
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -901,10 +901,7 @@ E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
 E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error);
 E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
 E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError);
-E('ERR_UNKNOWN_STDIN_TYPE', 'Unknown stdin file type', Error);
 
-// This should probably be a `TypeError`.
-E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type', Error);
 E('ERR_V8BREAKITERATOR',
   'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl',
   Error);
diff --git a/lib/internal/process/stdio.js b/lib/internal/process/stdio.js
@@ -1,10 +1,5 @@
 'use strict';
 
-const {
-  ERR_UNKNOWN_STDIN_TYPE,
-  ERR_UNKNOWN_STREAM_TYPE
-} = require('internal/errors').codes;
-
 exports.setupProcessStdio = setupProcessStdio;
 exports.getMainThreadStdio = getMainThreadStdio;
 
@@ -87,8 +82,11 @@ function getMainThreadStdio() {
         break;
 
       default:
-        // Probably an error on in uv_guess_handle()
-        throw new ERR_UNKNOWN_STDIN_TYPE();
+        // Provide a dummy contentless input for e.g. non-console
+        // Windows applications.
+        const { Readable } = require('stream');
+        stdin = new Readable({ read() {} });
+        stdin.push(null);
     }
 
     // For supporting legacy API we put the FD here.
@@ -123,6 +121,12 @@ function getMainThreadStdio() {
     return stdin;
   }
 
+  exports.resetStdioForTesting = function() {
+    stdin = undefined;
+    stdout = undefined;
+    stderr = undefined;
+  };
+
   return {
     getStdout,
     getStderr,
@@ -199,8 +203,14 @@ function createWritableStdioStream(fd) {
       break;
 
     default:
-      // Probably an error on in uv_guess_handle()
-      throw new ERR_UNKNOWN_STREAM_TYPE();
+      // Provide a dummy black-hole output for e.g. non-console
+      // Windows applications.
+      const { Writable } = require('stream');
+      stream = new Writable({
+        write(buf, enc, cb) {
+          cb();
+        }
+      });
   }
 
   // For supporting legacy API we put the FD here.
diff --git a/test/parallel/test-dummy-stdio.js b/test/parallel/test-dummy-stdio.js
@@ -0,0 +1,27 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const child_process = require('child_process');
+
+if (common.isWindows)
+  common.skip('fs.closeSync(n) does not close stdio on Windows');
+
+function runTest(fd, streamName, testOutputStream, expectedName) {
+  const result = child_process.spawnSync(process.execPath, [
+    '--expose-internals',
+    '-e', `
+    require('internal/process/stdio').resetStdioForTesting();
+    fs.closeSync(${fd});
+    const ctorName = process.${streamName}.constructor.name;
+    process.${testOutputStream}.write(ctorName);
+    `]);
+  assert.strictEqual(result[testOutputStream].toString(), expectedName,
+                     `stdout:\n${result.stdout}\nstderr:\n${result.stderr}\n` +
+                     `while running test with fd = ${fd}`);
+  if (testOutputStream !== 'stderr')
+    assert.strictEqual(result.stderr.toString(), '');
+}
+
+runTest(0, 'stdin', 'stdout', 'Readable');
+runTest(1, 'stdout', 'stderr', 'Writable');
+runTest(2, 'stderr', 'stdout', 'Writable');
