Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow to ignore routes based on req and res #64

Merged
merged 1 commit into from
Apr 9, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ Use `expressWinston.logger(options)` to create a middleware to log your HTTP req
meta: true, // optional: control whether you want to log the meta data about the request (default to true)
msg: "HTTP {{req.method}} {{req.url}}", // optional: customize the default logging message. E.g. "{{res.statusCode}} {{req.method}} {{res.responseTime}}ms {{req.url}}"
expressFormat: true, // Use the default Express/morgan request formatting, with the same colors. Enabling this will override any msg and colorStatus if true. Will only output colors on transports with colorize set to true
colorStatus: true // Color the status code, using the Express/morgan color palette (default green, 3XX cyan, 4XX yellow, 5XX red). Will not be recognized if expressFormat is true
colorStatus: true, // Color the status code, using the Express/morgan color palette (default green, 3XX cyan, 4XX yellow, 5XX red). Will not be recognized if expressFormat is true
ignoreRoute: function (req, res) { return false; } // optional: allows to skip some log messages based on request and/or response
}));

app.use(router); // notice how the router goes after the logger.
Expand Down
15 changes: 12 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ function errorLogger(options) {
function logger(options) {

ensureValidOptions(options);
ensureValidLoggerOptions(options);

options.requestFilter = options.requestFilter || defaultRequestFilter;
options.responseFilter = options.responseFilter || defaultResponseFilter;
Expand All @@ -144,15 +145,17 @@ function logger(options) {
options.msg = options.msg || "HTTP {{req.method}} {{req.url}}";
options.colorStatus = options.colorStatus || false;
options.expressFormat = options.expressFormat || false;
options.ignoreRoute = options.ignoreRoute || function () { return false; };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be making sure options.ignoreRoute is actually a function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I will add that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done... And I squashed the commit for the same price.


// Using mustache style templating
var template = _.template(options.msg, null, {
interpolate: /\{\{(.+?)\}\}/g
});

return function (req, res, next) {

if (req.route && req.route.path && _.contains(ignoredRoutes, req.route.path)) return next();
var currentUrl = req.originalUrl || req.url;
if (currentUrl && _.contains(ignoredRoutes, currentUrl)) return next();
if (options.ignoreRoute(req, res)) return next();

req._startTime = (new Date);

Expand Down Expand Up @@ -245,7 +248,13 @@ 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");
};
}

function ensureValidLoggerOptions(options) {
if (options.ignoreRoute && !_.isFunction(options.ignoreRoute)) {
throw new Error("`ignoreRoute` express-winston option should be a function");
}
}

module.exports.errorLogger = errorLogger;
module.exports.logger = logger;
Expand Down
49 changes: 49 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var should = require('should');
var util = require('util');
var _ = require('underscore');

var mocks = require('node-mocks-http');
var winston = require('winston');
Expand Down Expand Up @@ -216,6 +217,15 @@ describe('expressWinston', function () {
}).should.throwError();
});

it('should throw an error if ignoreRoute option is not a function', function () {
(function () {
expressWinston.logger({
transports: [new MockTransport({})],
ignoreRoute: 'not a function'
});
}).should.throwError();
});

it('should return a middleware function with three arguments that fit (req, res, next)', function () {
var middleware = expressWinston.logger({
transports: [new MockTransport({})]
Expand Down Expand Up @@ -460,6 +470,45 @@ describe('expressWinston', function () {
result.transportInvoked.should.eql(false);
});
});

describe('when invoked on a route that should be ignored (options.ignoreRoute)', function () {
var result;

before(function (done) {
setUp({
url: '/is-not-logged'
});
req.skip = true;
var test = {
req: req,
res: res,
log: {}
};

function next(_req, _res, next) {
res.end('{ "message": "Hi! I\'m a chunk!" }');
result = test;
return done();
};

var middleware = expressWinston.logger({
transports: [new MockTransport(test)],
ignoreRoute: function (req, res) {
return req.skip === true && req.url.match(/^\/is-not-log/);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's req.skip all about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was to illustrate that any property added to request (by a middleware for example) can be used in ignoreRoute... I guess it is not clear. I can just keep the req.url check.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that makes total sense. Cool.

}
});

middleware(req, res, next);
});

it('should not invoke the transport', function () {
result.transportInvoked.should.eql(false);
});

it('should contain a filtered request', function () {
result.log.should.be.empty;
});
});
});

describe('log.msg', function () {
Expand Down