Skip to content

Commit

Permalink
process: provide dummy stdio for non-console Windows apps
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax committed Jan 14, 2019
1 parent 272ddb1 commit 1805236
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 26 deletions.
36 changes: 22 additions & 14 deletions doc/api/errors.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1810,20 +1810,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 An invalid or unknown process signal was passed to an API expecting a valid
signal (such as [`subprocess.kill()`][]). 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> <a id="ERR_V8BREAKITERATOR"></a>
### ERR_V8BREAKITERATOR ### ERR_V8BREAKITERATOR


Expand Down Expand Up @@ -2080,6 +2066,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 code. Instances of this error point to an internal bug within the Node.js
binary itself. 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> <a id="ERR_VALUE_OUT_OF_RANGE"></a>
### ERR_VALUE_OUT_OF_RANGE ### ERR_VALUE_OUT_OF_RANGE
<!-- YAML <!-- YAML
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/errors.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -947,10 +947,7 @@ E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error); E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension: %s', Error);
E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError); E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s', RangeError);
E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError); 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', E('ERR_V8BREAKITERATOR',
'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl', 'Full ICU data not installed. See https://github.com/nodejs/node/wiki/Intl',
Error); Error);
Expand Down
28 changes: 19 additions & 9 deletions lib/internal/process/stdio.js
Original file line number Original file line Diff line number Diff line change
@@ -1,10 +1,5 @@
'use strict'; 'use strict';


const {
ERR_UNKNOWN_STDIN_TYPE,
ERR_UNKNOWN_STREAM_TYPE
} = require('internal/errors').codes;

exports.setupProcessStdio = setupProcessStdio; exports.setupProcessStdio = setupProcessStdio;
exports.getMainThreadStdio = getMainThreadStdio; exports.getMainThreadStdio = getMainThreadStdio;


Expand Down Expand Up @@ -87,8 +82,11 @@ function getMainThreadStdio() {
break; break;


default: default:
// Probably an error on in uv_guess_handle() // Provide a dummy contentless input for e.g. non-console
throw new ERR_UNKNOWN_STDIN_TYPE(); // Windows applications.
const { Readable } = require('stream');
stdin = new Readable({ read() {} });
stdin.push(null);
} }


// For supporting legacy API we put the FD here. // For supporting legacy API we put the FD here.
Expand Down Expand Up @@ -123,6 +121,12 @@ function getMainThreadStdio() {
return stdin; return stdin;
} }


exports.resetStdioForTesting = function() {
stdin = undefined;
stdout = undefined;
stderr = undefined;
};

return { return {
getStdout, getStdout,
getStderr, getStderr,
Expand Down Expand Up @@ -199,8 +203,14 @@ function createWritableStdioStream(fd) {
break; break;


default: default:
// Probably an error on in uv_guess_handle() // Provide a dummy black-hole output for e.g. non-console
throw new ERR_UNKNOWN_STREAM_TYPE(); // Windows applications.
const { Writable } = require('stream');
stream = new Writable({
write(buf, enc, cb) {
cb();
}
});
} }


// For supporting legacy API we put the FD here. // For supporting legacy API we put the FD here.
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-dummy-stdio.js
Original file line number Original file line Diff line number Diff line change
@@ -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');

0 comments on commit 1805236

Please sign in to comment.