From 5e9cac4333b7274d1354b86f44a43db374dac2f6 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 9 Mar 2016 15:26:34 -0800 Subject: [PATCH] console: check that stderr is writable `Console` constructor checks that `stdout.write()` is a function but does not do an equivalent check for `stderr.write()`. If `stderr` is not specified in the constructor, then `stderr` is set to be `stdout`. However, if `stderr` is specified, but `stderr.write()` is not a function, then an exception is not thrown until `console.error()` is called. This change adds the same check for 'stderr' in the constructor that is there for `stdout`. If `stderr` fails the check, then a `TypeError` is thrown. Took the opportunity to copyedit the `console` doc a little too. PR-URL: https://github.com/nodejs/node/pull/5635 Reviewed-By: James M Snell Reviewed-By: Rod Vagg --- doc/api/console.markdown | 8 ++++---- lib/console.js | 3 +++ test/parallel/test-console-instance.js | 19 +++++++++++++------ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/doc/api/console.markdown b/doc/api/console.markdown index f7d58c3e9f272e..6cf5051af2591e 100644 --- a/doc/api/console.markdown +++ b/doc/api/console.markdown @@ -48,8 +48,8 @@ myConsole.warn(`Danger ${name}! Danger!`); ``` While the API for the `Console` class is designed fundamentally around the -Web browser `console` object, the `Console` in Node.js is *not* intended to -duplicate the browsers functionality exactly. +browser `console` object, the `Console` in Node.js is *not* intended to +duplicate the browser's functionality exactly. ## Asynchronous vs Synchronous Consoles @@ -75,8 +75,8 @@ const Console = console.Console; Creates a new `Console` by passing one or two writable stream instances. `stdout` is a writable stream to print log or info output. `stderr` -is used for warning or error output. If `stderr` isn't passed, the warning -and error output will be sent to the `stdout`. +is used for warning or error output. If `stderr` isn't passed, warning and error +output will be sent to `stdout`. ```js const output = fs.createWriteStream('./stdout.log'); diff --git a/lib/console.js b/lib/console.js index 5137ae78c54079..15b483c248abc9 100644 --- a/lib/console.js +++ b/lib/console.js @@ -11,7 +11,10 @@ function Console(stdout, stderr) { } if (!stderr) { stderr = stdout; + } else if (typeof stderr.write !== 'function') { + throw new TypeError('Console expects writable stream instances'); } + var prop = { writable: true, enumerable: false, diff --git a/test/parallel/test-console-instance.js b/test/parallel/test-console-instance.js index 1ab038df48e432..2a8a6e3678f58a 100644 --- a/test/parallel/test-console-instance.js +++ b/test/parallel/test-console-instance.js @@ -1,10 +1,13 @@ 'use strict'; require('../common'); -var assert = require('assert'); -var Stream = require('stream'); -var Console = require('console').Console; +const assert = require('assert'); +const Stream = require('stream'); +const Console = require('console').Console; var called = false; +const out = new Stream(); +const err = new Stream(); + // ensure the Console instance doesn't write to the // process' "stdout" or "stderr" streams process.stdout.write = process.stderr.write = function() { @@ -20,9 +23,13 @@ assert.throws(function() { new Console(); }, /Console expects a writable stream/); -var out = new Stream(); -var err = new Stream(); -out.writable = err.writable = true; +// Console constructor should throw if stderr exists but is not writable +assert.throws(function() { + out.write = function() {}; + err.write = undefined; + new Console(out, err); +}, /Console expects writable stream instances/); + out.write = err.write = function(d) {}; var c = new Console(out, err);