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 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
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

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
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
Expand Down
3 changes: 0 additions & 3 deletions lib/internal/errors.js
Original file line number 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_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);
Expand Down
28 changes: 19 additions & 9 deletions lib/internal/process/stdio.js
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -123,6 +121,12 @@ function getMainThreadStdio() {
return stdin;
}

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

return {
getStdout,
getStderr,
Expand Down Expand Up @@ -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.
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-dummy-stdio.js
Original file line number 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.