Skip to content

Commit

Permalink
console: do not emit error events
Browse files Browse the repository at this point in the history
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 Feb 15, 2017
1 parent 0af4183 commit f18e08d
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 5 deletions.
58 changes: 53 additions & 5 deletions lib/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
Expand All @@ -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);
};


Expand All @@ -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.

Copy link
@Fishrock123

Fishrock123 Feb 17, 2017

Contributor

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

This comment has been minimized.

Copy link
@addaleax

addaleax Feb 17, 2017

Author Member

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

edit: I will

};


Expand Down Expand Up @@ -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() {}
17 changes: 17 additions & 0 deletions test/parallel/test-console-async-write-error.js
Original file line number Diff line number Diff line change
@@ -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');
});
47 changes: 47 additions & 0 deletions test/parallel/test-console-sync-write-error.js
Original file line number Diff line number Diff line change
@@ -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.