From cdc624272c8bdbca53bddd80ad062c6787fb4b0c Mon Sep 17 00:00:00 2001 From: DABH Date: Fri, 25 May 2018 00:36:53 -0700 Subject: [PATCH 1/4] console transport: send 'debug' level output to stdout not stderr by default --- .gitignore | 1 + UPGRADE-3.0.md | 1 + lib/winston/transports/console.js | 31 +++++++++++++++---------------- test/transports/console.test.js | 10 +++++----- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 8386c767b..4dd940187 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ coverage/* .nyc_output/* *.log .idea +*.sw* diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index c00b04a4f..9d79de8c9 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -26,6 +26,7 @@ ### Transports - `winston.transports.Memory` was removed. Use any Node.js `stream.Writeable` with a large `highWaterMark` instance instead. - When writing transports use `winston-transport` instead of `winston.Transport` +- In `winston.transports.Console`, log level 'debug' ouptut is now sent to stdout by default rather than stderr (controlled by `debugStdout` option) ### `winston.Container` and `winston.loggers` - `winston.Container` instances no longer have default `Console` transports diff --git a/lib/winston/transports/console.js b/lib/winston/transports/console.js index ffe756bb0..c0f1de192 100644 --- a/lib/winston/transports/console.js +++ b/lib/winston/transports/console.js @@ -78,30 +78,29 @@ module.exports = class Console extends TransportStream { * Array. For backwards compatibility, stderrLevels defaults to * ['error', 'debug'] or ['error'] depending on whether options.debugStdout * is true. - * @param {mixed} levels - TODO: add param description. - * @param {mixed} debugStdout - TODO: add param description. - * @returns {mixed} - TODO: add return description. + * @param {mixed} levels - An Array of string names of levels whose output + * should go to stderr + * @param {mixed} debugStdout - Whether the 'debug' level should be sent to + * stdout (default true as of `winston@3`). + * @returns {mixed} - An Object containing the levels to send to stderr. * @private */ _getStderrLevels(levels, debugStdout) { const defaultMsg = 'Cannot have non-string elements in stderrLevels Array'; - if (debugStdout) { - if (levels) { - // Don't allow setting both debugStdout and stderrLevels together, - // since this could cause behaviour a programmer might not expect. - throw new Error('Cannot set debugStdout and stderrLevels together'); + if (debugStdout && levels) { + // Don't allow setting both debugStdout and stderrLevels together, + // since this could cause behaviour a programmer might not expect. + throw new Error('Cannot set debugStdout and stderrLevels together'); + } else if (levels) { + if (!(Array.isArray(levels))) { + throw new Error('Cannot set stderrLevels to type other than Array'); } - - return this._stringArrayToSet(['error'], defaultMsg); - } - - if (!levels) { + return this._stringArrayToSet(levels, defaultMsg); + } else if (debugStdout === false) { return this._stringArrayToSet(['error', 'debug'], defaultMsg); - } else if (!(Array.isArray(levels))) { - throw new Error('Cannot set stderrLevels to type other than Array'); } - return this._stringArrayToSet(levels, defaultMsg); + return this._stringArrayToSet(['error'], defaultMsg); } /** diff --git a/test/transports/console.test.js b/test/transports/console.test.js index 26a328ec2..65706e6ea 100644 --- a/test/transports/console.test.js +++ b/test/transports/console.test.js @@ -56,7 +56,7 @@ function assertStderrLevels(transport, stderrLevels) { describe('Console transport', function () { describe('with defaults', function () { - it('logs all levels (EXCEPT error and debug) to stdout', function () { + it('logs all levels (EXCEPT error) to stdout', function () { stdMocks.use(); transports.defaults.levels = defaultLevels; Object.keys(defaultLevels) @@ -74,14 +74,14 @@ describe('Console transport', function () { stdMocks.restore(); var output = stdMocks.flush(); assume(output.stderr).is.an('array'); - assume(output.stderr).length(2); + assume(output.stderr).length(1); assume(output.stdout).is.an('array'); - assume(output.stdout).length(5); + assume(output.stdout).length(6); }); - it("should set stderrLevels to ['error', 'debug'] by default", assertStderrLevels( + it("should set stderrLevels to ['error'] by default", assertStderrLevels( transports.defaults, - ['error', 'debug'] + ['error'] )); }); From d3c249cf7e88e05f0f37553d79b5a259f15572be Mon Sep 17 00:00:00 2001 From: DABH Date: Fri, 25 May 2018 09:29:02 -0700 Subject: [PATCH 2/4] fix style --- lib/winston/transports/console.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/winston/transports/console.js b/lib/winston/transports/console.js index c0f1de192..0ba88f21b 100644 --- a/lib/winston/transports/console.js +++ b/lib/winston/transports/console.js @@ -92,7 +92,7 @@ module.exports = class Console extends TransportStream { // since this could cause behaviour a programmer might not expect. throw new Error('Cannot set debugStdout and stderrLevels together'); } else if (levels) { - if (!(Array.isArray(levels))) { + if (!(Array.isArray(levels))) { throw new Error('Cannot set stderrLevels to type other than Array'); } return this._stringArrayToSet(levels, defaultMsg); From 3938a7a9166e44cecd52cb1ea78f08291754e6cc Mon Sep 17 00:00:00 2001 From: DABH Date: Thu, 31 May 2018 14:35:20 -0700 Subject: [PATCH 3/4] Address PR comments / everything goes to stdout by default --- lib/winston/transports/console.js | 44 +++++++------------------------ test/transports/console.test.js | 30 +++++++-------------- 2 files changed, 18 insertions(+), 56 deletions(-) diff --git a/lib/winston/transports/console.js b/lib/winston/transports/console.js index 0ba88f21b..827f33eb5 100644 --- a/lib/winston/transports/console.js +++ b/lib/winston/transports/console.js @@ -28,10 +28,7 @@ module.exports = class Console extends TransportStream { // Expose the name of this Transport on the prototype this.name = 'console'; - this.stderrLevels = this._getStderrLevels( - options.stderrLevels, - options.debugStdout - ); + this.stderrLevels = this._stringArrayToSet(options.stderrLevels); this.eol = options.eol || os.EOL; } @@ -73,36 +70,6 @@ module.exports = class Console extends TransportStream { } } - /** - * Convert stderrLevels into an Object for faster key-lookup times than an - * Array. For backwards compatibility, stderrLevels defaults to - * ['error', 'debug'] or ['error'] depending on whether options.debugStdout - * is true. - * @param {mixed} levels - An Array of string names of levels whose output - * should go to stderr - * @param {mixed} debugStdout - Whether the 'debug' level should be sent to - * stdout (default true as of `winston@3`). - * @returns {mixed} - An Object containing the levels to send to stderr. - * @private - */ - _getStderrLevels(levels, debugStdout) { - const defaultMsg = 'Cannot have non-string elements in stderrLevels Array'; - if (debugStdout && levels) { - // Don't allow setting both debugStdout and stderrLevels together, - // since this could cause behaviour a programmer might not expect. - throw new Error('Cannot set debugStdout and stderrLevels together'); - } else if (levels) { - if (!(Array.isArray(levels))) { - throw new Error('Cannot set stderrLevels to type other than Array'); - } - return this._stringArrayToSet(levels, defaultMsg); - } else if (debugStdout === false) { - return this._stringArrayToSet(['error', 'debug'], defaultMsg); - } - - return this._stringArrayToSet(['error'], defaultMsg); - } - /** * Returns a Set-like object with strArray's elements as keys (each with the * value true). @@ -112,7 +79,14 @@ module.exports = class Console extends TransportStream { * @private */ _stringArrayToSet(strArray, errMsg) { - errMsg = errMsg || 'Cannot make set from Array with non-string elements'; + if (!strArray) + return {}; + + errMsg = errMsg || 'Cannot make set from type other than Array of string elements'; + + if (!Array.isArray(strArray)) { + throw new Error(errMsg); + } return strArray.reduce((set, el) => { if (typeof el !== 'string') { diff --git a/test/transports/console.test.js b/test/transports/console.test.js index 65706e6ea..da1edaa0e 100644 --- a/test/transports/console.test.js +++ b/test/transports/console.test.js @@ -19,7 +19,6 @@ const defaultLevels = winston.config.npm.levels; const transports = { defaults: new winston.transports.Console(), noStderr: new winston.transports.Console({ stderrLevels: [] }), - debugStdout: new winston.transports.Console({ debugStdout: true }), stderrLevels: new winston.transports.Console({ stderrLevels: ['info', 'warn'] }), @@ -56,7 +55,7 @@ function assertStderrLevels(transport, stderrLevels) { describe('Console transport', function () { describe('with defaults', function () { - it('logs all levels (EXCEPT error) to stdout', function () { + it('logs all levels to stdout', function () { stdMocks.use(); transports.defaults.levels = defaultLevels; Object.keys(defaultLevels) @@ -74,43 +73,32 @@ describe('Console transport', function () { stdMocks.restore(); var output = stdMocks.flush(); assume(output.stderr).is.an('array'); - assume(output.stderr).length(1); + assume(output.stderr).length(0); assume(output.stdout).is.an('array'); - assume(output.stdout).length(6); + assume(output.stdout).length(7); }); - it("should set stderrLevels to ['error'] by default", assertStderrLevels( + it("should set stderrLevels to [] by default", assertStderrLevels( transports.defaults, - ['error'] + [] )); }); describe('throws an appropriate error when', function () { - it('if both debugStdout and stderrLevels are set { debugStdout, stderrLevels }', function () { - assume(function () { - let throwing = new winston.transports.Console({ - stderrLevels: ['foo', 'bar'], - debugStdout: true - }) - }).throws(/Cannot set debugStdout and stderrLevels/); - }); - it("if stderrLevels is set, but not an Array { stderrLevels: 'Not an Array' }", function () { assume(function () { let throwing = new winston.transports.Console({ - stderrLevels: 'Not an Array', - debugStdout: false + stderrLevels: 'Not an Array' }) - }).throws(/Cannot set stderrLevels to type other than Array/); + }).throws(/Cannot make set from type other than Array of string elements/); }); it("if stderrLevels contains non-string elements { stderrLevels: ['good', /^invalid$/, 'valid']", function () { assume(function () { let throwing = new winston.transports.Console({ - stderrLevels: ['good', /^invalid$/, 'valid'], - debugStdout: false + stderrLevels: ['good', /^invalid$/, 'valid'] }) - }).throws(/Cannot have non-string elements in stderrLevels Array/); + }).throws(/Cannot make set from type other than Array of string elements/); }); }); From 710bb753f380c91ac9a140243a48793a029d029a Mon Sep 17 00:00:00 2001 From: DABH Date: Thu, 31 May 2018 14:37:02 -0700 Subject: [PATCH 4/4] update doc --- UPGRADE-3.0.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 9d79de8c9..5ef855d36 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -25,8 +25,10 @@ ### Transports - `winston.transports.Memory` was removed. Use any Node.js `stream.Writeable` with a large `highWaterMark` instance instead. -- When writing transports use `winston-transport` instead of `winston.Transport` -- In `winston.transports.Console`, log level 'debug' ouptut is now sent to stdout by default rather than stderr (controlled by `debugStdout` option) +- When writing transports use `winston-transport` instead of `winston.Transport`. +- In `winston.transports.Console`, output for all log levels is now sent to stdout by default. + - `debugStdout` option has been removed. + - `stderrLevels` now defaults to `[]`. ### `winston.Container` and `winston.loggers` - `winston.Container` instances no longer have default `Console` transports