diff --git a/Readme.md b/Readme.md index 994970a..9eb05b0 100644 --- a/Readme.md +++ b/Readme.md @@ -131,7 +131,11 @@ The logger needs to be added AFTER the express router(`app.router)`) and BEFORE To use winston's existing transports, set `transports` to the values (as in key-value) of the `winston.default.transports` object. This may be done, for example, by using underscorejs: `transports: _.values(winston.default.transports)`. -Alternatively, if you're using a winston logger instance elsewhere and have already set up levels and transports, pass the instance into expressWinston with the `winstonInstance` option. The `transports` option is then ignored. +Alternatively, if you're using a winston logger instance elsewhere and have already set up levels and transports, the `winstonInstance` option can be used. It has 2 forms: ++ an actual `winston` logger instance. In the case, the logger is used as is. ++ a factory function that takes parameters `req` and `res` of the middleware. The function is expected to return a `winston` logger instance. + +When using any version of `winstonInstance`, the `transports` option is then ignored. ## Examples diff --git a/index.js b/index.js index 9df1e84..2313a56 100644 --- a/index.js +++ b/index.js @@ -116,16 +116,15 @@ exports.errorLogger = function errorLogger(options) { options.requestWhitelist = options.requestWhitelist || exports.requestWhitelist; options.requestFilter = options.requestFilter || exports.defaultRequestFilter; - options.winstonInstance = options.winstonInstance || (winston.createLogger({ - transports: options.transports, - format: options.format - })); + const getLogger = loggerFactory(options); options.msg = options.msg || 'middlewareError'; options.baseMeta = options.baseMeta || {}; options.metaField = options.metaField || null; options.level = options.level || 'error'; options.dynamicMeta = options.dynamicMeta || function(req, res, err) { return null; }; - const exceptionHandler = new winston.ExceptionHandler(options.winstonInstance); + // TODO: improve the default exception meta information by not calling a + // constructor than a function that does nothing with the constructor parameter. + const exceptionHandler = new winston.ExceptionHandler({}); options.exceptionToMeta = options.exceptionToMeta || exceptionHandler.getAllInfo.bind(exceptionHandler); options.blacklistedMetaFields = options.blacklistedMetaFields || []; @@ -158,7 +157,7 @@ exports.errorLogger = function errorLogger(options) { options.msg = typeof options.msg === 'function' ? options.msg(req, res) : options.msg; // This is fire and forget, we don't want logging to hold up the request so don't wait for the callback - options.winstonInstance.log({ + getLogger(req, res).log({ level, message: getTemplate(options.msg, {err: err, req: req, res: res}), meta: exceptionMeta @@ -178,12 +177,35 @@ function levelFromStatus(options) { } } +/** + * returns a factory function to get a logger instance. + * The output function expect a request and result as parameters + * @param options - the library options + * @return func + */ +function loggerFactory(options) { + + if(options.winstonInstance) { + + if(_.isFunction(options.winstonInstance)) + return options.winstonInstance; + + const originalInstance = options.winstonInstance; + return function() { return originalInstance; }; + } + + const localLogger = winston.createLogger({ + transports: options.transports, + format: options.format + }); + return function() { return localLogger; }; +} + // // ### function logger(options) // #### @options {Object} options to initialize the middleware. // exports.logger = function logger(options) { - ensureValidOptions(options); ensureValidLoggerOptions(options); @@ -194,10 +216,8 @@ exports.logger = function logger(options) { options.requestFilter = options.requestFilter || exports.defaultRequestFilter; options.responseFilter = options.responseFilter || exports.defaultResponseFilter; options.ignoredRoutes = options.ignoredRoutes || exports.ignoredRoutes; - options.winstonInstance = options.winstonInstance || (winston.createLogger({ - transports: options.transports, - format: options.format - })); + + const getLogger = loggerFactory(options); options.statusLevels = options.statusLevels || false; options.level = options.statusLevels ? levelFromStatus(options) : (options.level || "info"); options.msg = options.msg || "HTTP {{req.method}} {{req.url}}"; @@ -346,7 +366,7 @@ exports.logger = function logger(options) { // This is fire and forget, we don't want logging to hold up the request so don't wait for the callback if (!options.skip(req, res)) { var level = _.isFunction(options.level) ? options.level(req, res) : options.level; - options.winstonInstance.log({level, message: msg, meta}); + getLogger(req, res).log({level, message: msg, meta}); } }; @@ -372,9 +392,11 @@ function bodyToString(body, isJSON) { function ensureValidOptions(options) { if(!options) throw new Error("options are required by express-winston middleware"); + if(!((options.transports && (options.transports.length > 0)) || options.winstonInstance)) throw new Error("transports or a winstonInstance are required by express-winston middleware"); + if (options.dynamicMeta && !_.isFunction(options.dynamicMeta)) { throw new Error("`dynamicMeta` express-winston option should be a function"); } diff --git a/test/test.js b/test/test.js index 8cbc3ec..60f1428 100644 --- a/test/test.js +++ b/test/test.js @@ -30,6 +30,16 @@ class MockTransport extends Transport { } } +class MockWinston { + constructor(test) { + this.invoked = false; + } + + log(options) { + this.invoked = true; + } +} + function mockReq(reqMock) { var reqSpec = _.extend({ method: 'GET', @@ -185,9 +195,84 @@ describe('express-winston', function () { }); }); - describe('when middleware function encounters an error in the pipeline', function () { - it('should invoke the transport', function () { - return errorLoggerTestHelper().then(function (result) { + describe('when providing logger instance', function() { + + it('should use the winstonInstance', function() { + + const winstonMock = new MockWinston(); + const localOptions = { + loggerOptions: { + winstonInstance: winstonMock, + transports: null + } + }; + + return errorLoggerTestHelper(localOptions).then(function(result) { + winstonMock.invoked.should.eql(true); + }); + }); + it('should use the winstonInstance in favor of transport specification', function() { + + const winstonMock = new MockWinston(); + const localOptions = { + loggerOptions: { + winstonInstance: winstonMock, + transports: [new MockTransport({})] + } + }; + + + return errorLoggerTestHelper(localOptions).then(function(result) { + winstonMock.invoked.should.eql(true); + }); + }); + + it('should invoke the winstonInstance factory if provided', function() { + const mockWinston = new MockWinston(); + + const loggerFactory = function() { + return mockWinston; + } + const localOptions = { + loggerOptions: { + winstonInstance: loggerFactory, + transports: null, + } + }; + return errorLoggerTestHelper(localOptions).then(function(result) { + mockWinston.invoked.should.eql(true); + }); + }); + + it('should have the req and res as parameters when invoking the winstonInstance factory', function() { + + const mockWinston = new MockWinston(); + let factoryReq; + let factoryRes; + const loggerFactory = function(req, res) { + factoryReq = req; + factoryRes = res; + return mockWinston; + } + const localOptions = { + loggerOptions: { + winstonInstance: loggerFactory, + transports: null, + } + }; + + return errorLoggerTestHelper(localOptions).then(function(result) { + mockWinston.invoked.should.eql(true); + factoryReq.should.be.deepEqual(result.req); + factoryRes.should.be.deepEqual(result.res); + }); + }); + + }); + + describe('when middleware function encounters an error in the pipeline', function() { + it('should invoke the transport', function() { + return errorLoggerTestHelper().then(function(result) { result.transportInvoked.should.eql(true); }); }); @@ -608,7 +693,83 @@ describe('express-winston', function () { }); }); - describe('when middleware function is invoked on a route', function () { + describe('when providing logger instance', function() { + + it('should use the winstonInstance', function() { + + const winstonMock = new MockWinston(); + const localOptions = { + loggerOptions: { + winstonInstance: winstonMock, + transports: null + } + }; + + return loggerTestHelper(localOptions).then(function(result) { + winstonMock.invoked.should.eql(true); + }); + }); + it('should use the winstonInstance in favor of transport specification', function() { + + const winstonMock = new MockWinston(); + const localOptions = { + loggerOptions: { + winstonInstance: winstonMock, + transports: [new MockTransport({})] + } + }; + + + return loggerTestHelper(localOptions).then(function(result) { + winstonMock.invoked.should.eql(true); + }); + }); + + it('should invoke the winstonInstance factory if provided', function() { + const mockWinston = new MockWinston(); + + const loggerFactory = function() { + return mockWinston; + } + const localOptions = { + loggerOptions: { + winstonInstance: loggerFactory, + transports: null, + } + }; + return loggerTestHelper(localOptions).then(function(result) { + mockWinston.invoked.should.eql(true); + }); + }); + + it('should have the req and res as parameters when invoking the winstonInstance factory', function() { + + const mockWinston = new MockWinston(); + let factoryReq; + let factoryRes; + const loggerFactory = function(req, res) { + factoryReq = req; + factoryRes = res; + return mockWinston; + } + const localOptions = { + loggerOptions: { + winstonInstance: loggerFactory, + transports: null, + } + }; + + return loggerTestHelper(localOptions).then(function(result) { + mockWinston.invoked.should.eql(true); + factoryReq.should.be.deepEqual(result.req); + factoryRes.should.be.deepEqual(result.res); + }); + }); + + }); + + + describe('when middleware function is invoked on a route', function() { function next(req, res, next) { req._startTime = (new Date()) - 125;