Permalink
Browse files

console: do not emit error events

Fixes: #831
Fixes: #947
Ref: #9470
PR-URL: #9744
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
  • Loading branch information...
addaleax committed Nov 22, 2016
1 parent 0af4183 commit f18e08d820dde161788d95a5603546ceca021e90
View
@@ -2,9 +2,9 @@
const util = require('util');
function Console(stdout, stderr) {
function Console(stdout, stderr, ignoreErrors = true) {
if (!(this instanceof Console)) {
return new Console(stdout, stderr);
return new Console(stdout, stderr, ignoreErrors);
}
if (!stdout || typeof stdout.write !== 'function') {
throw new TypeError('Console expects a writable stream instance');
@@ -24,8 +24,14 @@ function Console(stdout, stderr) {
Object.defineProperty(this, '_stdout', prop);
prop.value = stderr;
Object.defineProperty(this, '_stderr', prop);
prop.value = ignoreErrors;
Object.defineProperty(this, '_ignoreErrors', prop);
prop.value = new Map();
Object.defineProperty(this, '_times', prop);
prop.value = createWriteErrorHandler(stdout);
Object.defineProperty(this, '_stdoutErrorHandler', prop);
prop.value = createWriteErrorHandler(stderr);
Object.defineProperty(this, '_stderrErrorHandler', prop);
// bind the prototype functions to this Console instance
var keys = Object.keys(Console.prototype);
@@ -35,20 +41,60 @@ function Console(stdout, stderr) {
}
}
// Make a function that can serve as the callback passed to `stream.write()`.
function createWriteErrorHandler(stream) {
return (err) => {
// This conditional evaluates to true if and only if there was an error
// that was not already emitted (which happens when the _write callback
// is invoked asynchronously).
if (err && !stream._writableState.errorEmitted) {
// If there was an error, it will be emitted on `stream` as
// an `error` event. Adding a `once` listener will keep that error
// from becoming an uncaught exception, but since the handler is
// removed after the event, non-console.* writes won’t be affected.
stream.once('error', noop);
}
};
}
function write(ignoreErrors, stream, string, errorhandler) {
if (!ignoreErrors) return stream.write(string);
// There may be an error occurring synchronously (e.g. for files or TTYs
// on POSIX systems) or asynchronously (e.g. pipes on POSIX systems), so
// handle both situations.
try {
// Add and later remove a noop error handler to catch synchronous errors.
stream.once('error', noop);
stream.write(string, errorhandler);
} catch (e) {
// Sorry, there’s no proper way to pass along the error here.
} finally {
stream.removeListener('error', noop);
}
}
// As of v8 5.0.71.32, the combination of rest param, template string
// and .apply(null, args) benchmarks consistently faster than using
// the spread operator when calling util.format.
Console.prototype.log = function log(...args) {
this._stdout.write(`${util.format.apply(null, args)}\n`);
write(this._ignoreErrors,
this._stdout,
`${util.format.apply(null, args)}\n`,
this._stdoutErrorHandler);
};
Console.prototype.info = Console.prototype.log;
Console.prototype.warn = function warn(...args) {
this._stderr.write(`${util.format.apply(null, args)}\n`);
write(this._ignoreErrors,
this._stderr,
`${util.format.apply(null, args)}\n`,
this._stderrErrorHandler);
};
@@ -57,7 +103,7 @@ Console.prototype.error = Console.prototype.warn;
Console.prototype.dir = function dir(object, options) {
options = Object.assign({customInspect: false}, options);
this._stdout.write(`${util.inspect(object, options)}\n`);
write(this._ignoreErrors, this._stdout, `${util.inspect(object, options)}\n`);

This comment has been minimized.

Show comment
Hide comment
@Fishrock123

Fishrock123 Feb 17, 2017

Member

@addaleax why doesn't this pass the this._stdoutErrorHandler?

@Fishrock123

Fishrock123 Feb 17, 2017

Member

@addaleax why doesn't this pass the this._stdoutErrorHandler?

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Feb 17, 2017

Member

@Fishrock123 … yeah, overlooked. Do you want to open a PR or should I?

edit: I will

@addaleax

addaleax Feb 17, 2017

Member

@Fishrock123 … yeah, overlooked. Do you want to open a PR or should I?

edit: I will

};
@@ -99,3 +145,5 @@ Console.prototype.assert = function assert(expression, ...args) {
module.exports = new Console(process.stdout, process.stderr);
module.exports.Console = Console;
function noop() {}
@@ -0,0 +1,17 @@
'use strict';
const common = require('../common');
const { Console } = require('console');
const { Writable } = require('stream');
const assert = require('assert');
const out = new Writable({
write: common.mustCall((chunk, enc, callback) => {
process.nextTick(callback, new Error('foobar'));
})
});
const c = new Console(out, out, true);
assert.doesNotThrow(() => {
c.log('abc');
});
@@ -0,0 +1,47 @@
'use strict';
const common = require('../common');
const { Console } = require('console');
const { Writable } = require('stream');
const assert = require('assert');
{
const out = new Writable({
write: common.mustCall((chunk, enc, callback) => {
callback(new Error('foobar'));
})
});
const c = new Console(out, out, true);
assert.doesNotThrow(() => {
c.log('abc');
});
}
{
const out = new Writable({
write: common.mustCall((chunk, enc, callback) => {
throw new Error('foobar');
})
});
const c = new Console(out, out, true);
assert.doesNotThrow(() => {
c.log('abc');
});
}
{
const out = new Writable({
write: common.mustCall((chunk, enc, callback) => {
setImmediate(() => callback(new Error('foobar')));
})
});
const c = new Console(out, out, true);
assert.doesNotThrow(() => {
c.log('abc');
});
}

0 comments on commit f18e08d

Please sign in to comment.