Feature/logger factory

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
 

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");
     }

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;