From 22ac48396a7ef87c832bad6c6016a60ce464a03a Mon Sep 17 00:00:00 2001 From: Ryan Day Date: Wed, 26 Sep 2018 13:32:59 -0700 Subject: [PATCH 1/3] fix: assign log levels to un bound functions binding custom log level functions to `this` with arrow functions causes unintentional behavior. the expectation is that they should be callable the same way as logger.log if you extend a logger instance `logger.log` will be called on `logger` whereas `logger.warn` will be called on `logger.prototype` this fixes #1482 but I believe it should be treated as a bug. I doubt blocking extension of only part of the surface area of the instance was intended. this sets the base for efficient child loggers that only extend write to add attributes. --- lib/winston/create-logger.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/winston/create-logger.js b/lib/winston/create-logger.js index c17780640..e426a60eb 100644 --- a/lib/winston/create-logger.js +++ b/lib/winston/create-logger.js @@ -45,7 +45,8 @@ class DerivedLogger extends Logger { // Define prototype methods for each log level // e.g. logger.log('info', msg) <––> logger.info(msg) & logger.isInfoEnabled() - this[level] = (...args) => { + // this is not an arrow function so it'll always be called on the instance not at a fixed place in the prototype chain. + this[level] = function(...args){ // Optimize the hot-path which is the single object. if (args.length === 1) { const [msg] = args; From 4f3608211b16e8e32a5765c85e9fa4d8f7b282b9 Mon Sep 17 00:00:00 2001 From: Ryan Day Date: Wed, 26 Sep 2018 14:09:12 -0700 Subject: [PATCH 2/3] test: test for custom method prototype binding --- lib/winston/create-logger.js | 4 ++-- test/logger.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/lib/winston/create-logger.js b/lib/winston/create-logger.js index e426a60eb..fedfadaa4 100644 --- a/lib/winston/create-logger.js +++ b/lib/winston/create-logger.js @@ -45,8 +45,8 @@ class DerivedLogger extends Logger { // Define prototype methods for each log level // e.g. logger.log('info', msg) <––> logger.info(msg) & logger.isInfoEnabled() - // this is not an arrow function so it'll always be called on the instance not at a fixed place in the prototype chain. - this[level] = function(...args){ + // this is not an arrow function so it'll always be called on the instance instead of a fixed place in the prototype chain. + this[level] = function (...args) { // Optimize the hot-path which is the single object. if (args.length === 1) { const [msg] = args; diff --git a/test/logger.test.js b/test/logger.test.js index 48fa31bb4..dbc3b2063 100755 --- a/test/logger.test.js +++ b/test/logger.test.js @@ -66,6 +66,33 @@ describe('Logger', function () { }) }); + it('new Logger({ levels }) custom methods are not bound to instance', function () { + + var logger = winston.createLogger({ + level: 'error', + exitOnError: false, + transports: [] + }); + + let logs = [] + + let extendedLogger = Object.create(logger,{ + write:{ + value:function(...args){ + logs.push(args) + } + } + }) + + extendedLogger.log({test:1}) + extendedLogger.warn({test:2}) + + assume(logs[0]||[]).is.eql([{test:1}]) + assume(logs[1]||[]).is.eql([{message:{test:2},level:'warn'}]) + + + }); + it('.add({ invalid Transport })', function () { var logger = winston.createLogger(); assume(function () { From 907a885c3b3b1b3c425fd8262b2abf08a10acebb Mon Sep 17 00:00:00 2001 From: DABH Date: Wed, 26 Sep 2018 14:52:18 -0700 Subject: [PATCH 3/3] style --- test/logger.test.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/test/logger.test.js b/test/logger.test.js index dbc3b2063..da0beca28 100755 --- a/test/logger.test.js +++ b/test/logger.test.js @@ -67,30 +67,27 @@ describe('Logger', function () { }); it('new Logger({ levels }) custom methods are not bound to instance', function () { - var logger = winston.createLogger({ level: 'error', exitOnError: false, transports: [] }); - let logs = [] + let logs = []; let extendedLogger = Object.create(logger,{ write:{ value:function(...args){ - logs.push(args) + logs.push(args); } } - }) - - extendedLogger.log({test:1}) - extendedLogger.warn({test:2}) - - assume(logs[0]||[]).is.eql([{test:1}]) - assume(logs[1]||[]).is.eql([{message:{test:2},level:'warn'}]) + }); + extendedLogger.log({test:1}); + extendedLogger.warn({test:2}); + assume(logs[0]||[]).is.eql([{test:1}]); + assume(logs[1]||[]).is.eql([{message:{test:2},level:'warn'}]); }); it('.add({ invalid Transport })', function () {