Skip to content

Commit ce7c951

Browse files
indexzeroindexzero
authored
Attempt to fix #1577. (#1579)
* [fix wip] Attempt to fix #1577. * [fix test] Fallback to the "root" instance **always** created by `createLogger` for level convenience methods (e.g. `.info()`, `.silly()`). * [tiny] Remove useless assignment. * [tiny] DRY it up a little.
1 parent 3c4d5b0 commit ce7c951

File tree

2 files changed

+106
-71
lines changed

2 files changed

+106
-71
lines changed

‎lib/winston/create-logger.js

Lines changed: 78 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,79 +12,93 @@ const config = require('./config');
1212
const Logger = require('./logger');
1313
const debug = require('diagnostics')('winston:create-logger');
1414

15+
function isLevelEnabledFunctionName(level) {
16+
return 'is' + level.charAt(0).toUpperCase() + level.slice(1) + 'Enabled';
17+
}
18+
1519
/**
16-
* DerivedLogger to attach the logs level methods.
17-
* @type {DerivedLogger}
18-
* @extends {Logger}
20+
* Create a new instance of a winston Logger. Creates a new
21+
* prototype for each instance.
22+
* @param {!Object} opts - Options for the created logger.
23+
* @returns {Logger} - A newly created logger instance.
1924
*/
20-
class DerivedLogger extends Logger {
25+
module.exports = function (opts = {}) {
26+
//
27+
// Default levels: npm
28+
//
29+
opts.levels = opts.levels || config.npm.levels;
30+
2131
/**
22-
* Create a new class derived logger for which the levels can be attached to
23-
* the prototype of. This is a V8 optimization that is well know to increase
24-
* performance of prototype functions.
25-
* @param {!Object} options - Options for the created logger.
32+
* DerivedLogger to attach the logs level methods.
33+
* @type {DerivedLogger}
34+
* @extends {Logger}
2635
*/
27-
constructor(options) {
28-
super(options);
29-
this._setupLevels();
36+
class DerivedLogger extends Logger {
37+
/**
38+
* Create a new class derived logger for which the levels can be attached to
39+
* the prototype of. This is a V8 optimization that is well know to increase
40+
* performance of prototype functions.
41+
* @param {!Object} options - Options for the created logger.
42+
*/
43+
constructor(options) {
44+
super(options);
45+
}
3046
}
3147

32-
/**
33-
* Create the log level methods for the derived logger.
34-
* @returns {undefined}
35-
* @private
36-
*/
37-
_setupLevels() {
38-
Object.keys(this.levels).forEach(level => {
39-
debug('Define prototype method for "%s"', level);
40-
if (level === 'log') {
41-
// eslint-disable-next-line no-console
42-
console.warn('Level "log" not defined: conflicts with the method "log". Use a different level name.');
43-
return;
44-
}
48+
const logger = new DerivedLogger(opts);
49+
50+
//
51+
// Create the log level methods for the derived logger.
52+
//
53+
Object.keys(opts.levels).forEach(function (level) {
54+
debug('Define prototype method for "%s"', level);
55+
if (level === 'log') {
56+
// eslint-disable-next-line no-console
57+
console.warn('Level "log" not defined: conflicts with the method "log". Use a different level name.');
58+
return;
59+
}
4560

46-
// Define prototype methods for each log level
47-
// e.g. logger.log('info', msg) <––> logger.info(msg) & logger.isInfoEnabled()
48-
// this is not an arrow function so it'll always be called on the instance instead of a fixed place in the prototype chain.
49-
this[level] = function (...args) {
50-
// Optimize the hot-path which is the single object.
51-
if (args.length === 1) {
52-
const [msg] = args;
53-
const info = msg && msg.message && msg || { message: msg };
54-
info.level = info[LEVEL] = level;
55-
this._addDefaultMeta(info);
56-
this.write(info);
57-
return this;
58-
}
61+
//
62+
// Define prototype methods for each log level e.g.:
63+
// logger.log('info', msg) implies these methods are defined:
64+
// - logger.info(msg)
65+
// - logger.isInfoEnabled()
66+
//
67+
// Remark: to support logger.child this **MUST** be a function
68+
// so it'll always be called on the instance instead of a fixed
69+
// place in the prototype chain.
70+
//
71+
DerivedLogger.prototype[level] = function (...args) {
72+
// Prefer any instance scope, but default to "root" logger
73+
const self = this || logger;
5974

60-
// When provided nothing assume the empty string
61-
if (args.length === 0) {
62-
this.log(level, '');
63-
return this;
64-
}
75+
// Optimize the hot-path which is the single object.
76+
if (args.length === 1) {
77+
const [msg] = args;
78+
const info = msg && msg.message && msg || { message: msg };
79+
info.level = info[LEVEL] = level;
80+
self._addDefaultMeta(info);
81+
self.write(info);
82+
return (this || logger);
83+
}
6584

66-
// Otherwise build argument list which could potentially conform to
67-
// either:
68-
// . v3 API: log(obj)
69-
// 2. v1/v2 API: log(level, msg, ... [string interpolate], [{metadata}], [callback])
70-
return this.log(level, ...args);
71-
};
85+
// When provided nothing assume the empty string
86+
if (args.length === 0) {
87+
self.log(level, '');
88+
return self;
89+
}
7290

73-
this[isLevelEnabledFunctionName(level)] = () => this.isLevelEnabled(level);
74-
});
75-
}
76-
}
91+
// Otherwise build argument list which could potentially conform to
92+
// either:
93+
// . v3 API: log(obj)
94+
// 2. v1/v2 API: log(level, msg, ... [string interpolate], [{metadata}], [callback])
95+
return self.log(level, ...args);
96+
};
7797

78-
function isLevelEnabledFunctionName(level) {
79-
return 'is' + level.charAt(0).toUpperCase() + level.slice(1) + 'Enabled';
80-
}
98+
DerivedLogger.prototype[isLevelEnabledFunctionName(level)] = function () {
99+
return (this || logger).isLevelEnabled(level);
100+
};
101+
});
81102

82-
/**
83-
* Create a new instance of a winston Logger. Creates a new
84-
* prototype for each instance.
85-
* @param {!Object} opts - Options for the created logger.
86-
* @returns {Logger} - A newly created logger instance.
87-
*/
88-
module.exports = (opts = { levels: config.npm.levels }) => (
89-
new DerivedLogger(opts)
90-
);
103+
return logger;
104+
};

‎test/logger.test.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('Logger', function () {
6767
})
6868
});
6969

70-
it('new Logger({ levels }) custom methods are not bound to instance', function () {
70+
it('new Logger({ levels }) custom methods are not bound to instance', function (done) {
7171
var logger = winston.createLogger({
7272
level: 'error',
7373
exitOnError: false,
@@ -79,16 +79,27 @@ describe('Logger', function () {
7979
write: {
8080
value: function(...args) {
8181
logs.push(args);
82+
if (logs.length === 4) {
83+
assume(logs.length).is.eql(4);
84+
assume(logs[0]).is.eql([{ test: 1, level: 'info' }]);
85+
assume(logs[1]).is.eql([{ test: 2, level: 'warn' }]);
86+
assume(logs[2]).is.eql([{ message: 'test3', level: 'info' }])
87+
assume(logs[3]).is.eql([{ with: 'meta',
88+
test: 4,
89+
level: 'warn',
90+
message: 'a warning'
91+
}]);
92+
93+
done();
94+
}
8295
}
8396
}
8497
});
8598

86-
extendedLogger.log({ test:1 });
87-
extendedLogger.warn({ test:2 });
88-
89-
assume(logs.length).is.eql(2);
90-
assume(logs[0] || []).is.eql([{ test:1 }]);
91-
assume(logs[1] || []).is.eql([{ message: { test:2 }, level: 'warn' }]);
99+
extendedLogger.log('info', { test: 1 });
100+
extendedLogger.log('warn', { test: 2 });
101+
extendedLogger.info('test3');
102+
extendedLogger.warn('a warning', { with: 'meta', test: 4 });
92103
});
93104

94105
it('.add({ invalid Transport })', function () {
@@ -1057,4 +1068,14 @@ describe('Should support child loggers & defaultMeta', () => {
10571068
const childLogger = logger.child({ service: 'user-service' });
10581069
childLogger.error(Error('dummy error'));
10591070
});
1071+
1072+
it('defaultMeta() autobinds correctly', (done) => {
1073+
const logger = helpers.createLogger(info => {
1074+
assume(info.message).equals('test');
1075+
done();
1076+
});
1077+
1078+
const log = logger.info;
1079+
log('test');
1080+
});
10601081
});

0 commit comments

Comments
 (0)