From 373402f5c4750f3dd8b89580c2cca8cd41e0ffdc Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 18 Oct 2016 19:23:55 +0100 Subject: [PATCH] process: swallow stdout/stderr error events This is a step in making `console` safe, swallows EPIPE and other errors by registering an empty 'error' event listener. Also fixes writes to stdout/stderr not working after `end()` is called, even though we already override `destroy()` so the stream is never actually closed. Refs: https://github.com/nodejs/node/issues/831 Fixes: https://github.com/nodejs/node/issues/947 Fixes: https://github.com/nodejs/node/issues/9403 --- lib/internal/process/stdio.js | 38 +++++++++++++++++++ .../test-process-external-stdio-close.js | 0 ...out-cannot-be-closed-child-process-pipe.js | 4 +- test/pseudo-tty/test-tty-stdout-end.js | 17 +++------ 4 files changed, 46 insertions(+), 13 deletions(-) rename test/{known_issues => parallel}/test-process-external-stdio-close.js (100%) diff --git a/lib/internal/process/stdio.js b/lib/internal/process/stdio.js index c8f36c5df60c94..4b083cc3e433b2 100644 --- a/lib/internal/process/stdio.js +++ b/lib/internal/process/stdio.js @@ -8,26 +8,64 @@ function setupStdio() { function getStdout() { if (stdout) return stdout; stdout = createWritableStdioStream(1); + + Object.defineProperty(stdout._writableState, 'ended', { + set() {}, + get() { + return false; + } + }); + // XXX(Fishrock123): This should work I think, but it causes + // problems with some child-process related tests. + // + // Object.defineProperty(stdout, 'destroyed', { + // set() {}, + // get() { + // return true; + // } + // }); + stdout.destroy = stdout.destroySoon = function(er) { + stdout.destroyed = true; er = er || new Error('process.stdout cannot be closed.'); stdout.emit('error', er); }; if (stdout.isTTY) { process.on('SIGWINCH', () => stdout._refreshSize()); } + stdout.on('error', () => {}); return stdout; } function getStderr() { if (stderr) return stderr; stderr = createWritableStdioStream(2); + + Object.defineProperty(stderr._writableState, 'ended', { + set() {}, + get() { + return false; + } + }); + // XXX(Fishrock123): This should work I think, but it causes + // problems with some child-process related tests. + // + // Object.defineProperty(stderr, 'destroyed', { + // set() {}, + // get() { + // return true; + // } + // }); + stderr.destroy = stderr.destroySoon = function(er) { + stderr.destroyed = true; er = er || new Error('process.stderr cannot be closed.'); stderr.emit('error', er); }; if (stderr.isTTY) { process.on('SIGWINCH', () => stderr._refreshSize()); } + stderr.on('error', () => {}); return stderr; } diff --git a/test/known_issues/test-process-external-stdio-close.js b/test/parallel/test-process-external-stdio-close.js similarity index 100% rename from test/known_issues/test-process-external-stdio-close.js rename to test/parallel/test-process-external-stdio-close.js diff --git a/test/parallel/test-stdout-cannot-be-closed-child-process-pipe.js b/test/parallel/test-stdout-cannot-be-closed-child-process-pipe.js index a138108fae7e01..8e2fc33b2fea09 100644 --- a/test/parallel/test-stdout-cannot-be-closed-child-process-pipe.js +++ b/test/parallel/test-stdout-cannot-be-closed-child-process-pipe.js @@ -24,9 +24,9 @@ function parent() { }); child.on('close', function(code, signal) { - assert(code); + assert.strictEqual(code, 0, 'exit code should be zero'); assert.equal(out, 'foo'); - assert(/process\.stdout cannot be closed/.test(err)); + assert.strictEqual(err, '', 'stderr should be empty'); console.log('ok'); }); } diff --git a/test/pseudo-tty/test-tty-stdout-end.js b/test/pseudo-tty/test-tty-stdout-end.js index 86a42c4035b7ae..1aab200c76a17f 100644 --- a/test/pseudo-tty/test-tty-stdout-end.js +++ b/test/pseudo-tty/test-tty-stdout-end.js @@ -1,14 +1,9 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); -const shouldThrow = function() { - process.stdout.end(); -}; - -const validateError = function(e) { - return e instanceof Error && - e.message === 'process.stdout cannot be closed.'; -}; - -assert.throws(shouldThrow, validateError); +process.stdout.on('error', common.mustCall(function(e) { + assert(e instanceof Error, 'e is an Error'); + assert.strictEqual(e.message, 'process.stdout cannot be closed.'); +})); +process.stdout.end();