diff --git a/lib/connect-logger.js b/lib/connect-logger.js index c250ac0..9ed953c 100755 --- a/lib/connect-logger.js +++ b/lib/connect-logger.js @@ -146,20 +146,18 @@ function format(str, tokens) { function createNoLogCondition(nolog) { let regexp = null; - if (nolog) { - if (nolog instanceof RegExp) { - regexp = nolog; - } + if (nolog instanceof RegExp) { + regexp = nolog; + } - if (typeof nolog === 'string') { - regexp = new RegExp(nolog); - } + if (typeof nolog === 'string') { + regexp = new RegExp(nolog); + } - if (Array.isArray(nolog)) { - // convert to strings - const regexpsAsStrings = nolog.map(reg => (reg.source ? reg.source : reg)); - regexp = new RegExp(regexpsAsStrings.join('|')); - } + if (Array.isArray(nolog)) { + // convert to strings + const regexpsAsStrings = nolog.map(reg => (reg.source ? reg.source : reg)); + regexp = new RegExp(regexpsAsStrings.join('|')); } return regexp; @@ -189,7 +187,7 @@ function matchRules(statusCode, currentLevel, ruleSet) { let ruleMatched = false; if (rule.from && rule.to) { ruleMatched = statusCode >= rule.from && statusCode <= rule.to; - } else if (rule.codes) { + } else { ruleMatched = rule.codes.indexOf(statusCode) !== -1; } return ruleMatched; @@ -232,18 +230,16 @@ function matchRules(statusCode, currentLevel, ruleSet) { */ module.exports = function getLogger(logger4js, options) { /* eslint no-underscore-dangle:0 */ - if (typeof options === 'object') { - options = options || {}; - } else if (options) { + if (typeof options === 'string' || typeof options === 'function') { options = { format: options }; } else { - options = {}; + options = options || {}; } const thisLogger = logger4js; let level = levels.getLevel(options.level, levels.INFO); const fmt = options.format || DEFAULT_FORMAT; - const nolog = options.nolog ? createNoLogCondition(options.nolog) : null; + const nolog = createNoLogCondition(options.nolog); return (req, res, next) => { // mount safety @@ -289,15 +285,13 @@ module.exports = function getLogger(logger4js, options) { } level = matchRules(res.statusCode, level, options.statusRules); - if (thisLogger.isLevelEnabled(level)) { - const combinedTokens = assembleTokens(req, res, options.tokens || []); + const combinedTokens = assembleTokens(req, res, options.tokens || []); - if (typeof fmt === 'function') { - const line = fmt(req, res, str => format(str, combinedTokens)); - if (line) thisLogger.log(level, line); - } else { - thisLogger.log(level, format(fmt, combinedTokens)); - } + if (typeof fmt === 'function') { + const line = fmt(req, res, str => format(str, combinedTokens)); + if (line) thisLogger.log(level, line); + } else { + thisLogger.log(level, format(fmt, combinedTokens)); } }); } diff --git a/test/tap/connect-logger-test.js b/test/tap/connect-logger-test.js index 611a7f5..8450080 100644 --- a/test/tap/connect-logger-test.js +++ b/test/tap/connect-logger-test.js @@ -19,9 +19,10 @@ class MockLogger { } } -function MockRequest(remoteAddr, method, originalUrl, headers) { +function MockRequest(remoteAddr, method, originalUrl, headers, url) { this.socket = { remoteAddress: remoteAddr }; this.originalUrl = originalUrl; + this.url = url; this.method = method; this.httpVersionMajor = '5'; this.httpVersionMinor = '0'; @@ -48,11 +49,15 @@ class MockResponse extends EE { } } -function request(cl, method, url, code, reqHeaders, resHeaders) { - const req = new MockRequest('my.remote.addr', method, url, reqHeaders); +function request(cl, method, originalUrl, code, reqHeaders, resHeaders, next, url) { + const req = new MockRequest('my.remote.addr', method, originalUrl, reqHeaders, url); const res = new MockResponse(); - cl(req, res, () => { - }); + if (next) { + next = next.bind(null, req, res, () => {}); + } else { + next = () => {}; + } + cl(req, res, next); res.writeHead(code, resHeaders); res.end('chunk', 'encoding'); } @@ -100,16 +105,32 @@ test('log4js connect logger', (batch) => { t.test('log events with non-default level and custom format', (assert) => { const ml = new MockLogger(); ml.level = levels.INFO; - const cl = clm(ml, { level: levels.INFO, format: ':method :url' }); + const cl = clm(ml, { level: levels.WARN, format: ':method :url' }); request(cl, 'GET', 'http://url', 200); const messages = ml.messages; assert.type(messages, Array); assert.equal(messages.length, 1); - assert.ok(levels.INFO.isEqualTo(messages[0].level)); + assert.ok(levels.WARN.isEqualTo(messages[0].level)); assert.equal(messages[0].message, 'GET http://url'); assert.end(); }); + + t.test('adding multiple loggers should only log once', (assert) => { + const ml = new MockLogger(); + ml.level = levels.INFO; + const cl = clm(ml, { level: levels.WARN, format: ':method :url' }); + const nextLogger = clm(ml, { level: levels.INFO, format: ':method' }); + request(cl, 'GET', 'http://url', 200, null, null, nextLogger); + + const messages = ml.messages; + assert.type(messages, Array); + assert.equal(messages.length, 1); + assert.ok(levels.WARN.isEqualTo(messages[0].level)); + assert.equal(messages[0].message, 'GET http://url'); + + assert.end(); + }); t.end(); }); @@ -209,6 +230,26 @@ test('log4js connect logger', (batch) => { t.end(); }); + batch.test('format using a function that also uses tokens', (t) => { + const ml = new MockLogger(); + ml.level = levels.INFO; + const cl = clm(ml, (req, res, tokenReplacer) => `${req.method} ${tokenReplacer(':status')}`); + request(cl, 'GET', 'http://blah', 200); + + t.equal(ml.messages[0].message, 'GET 200'); + t.end(); + }); + + batch.test('format using a function, but do not log anything if the function returns nothing', (t) => { + const ml = new MockLogger(); + ml.level = levels.INFO; + const cl = clm(ml, () => null); + request(cl, 'GET', 'http://blah', 200); + + t.equal(ml.messages.length, 0); + t.end(); + }); + batch.test('format that includes request headers', (t) => { const ml = new MockLogger(); ml.level = levels.INFO; @@ -238,6 +279,15 @@ test('log4js connect logger', (batch) => { t.end(); }); + batch.test('url token should check originalUrl and url', (t) => { + const ml = new MockLogger(); + const cl = clm(ml, ':url'); + request(cl, 'GET', null, 200, null, null, null, 'http://cheese'); + + t.equal(ml.messages[0].message, 'http://cheese'); + t.end(); + }); + batch.test('log events with custom token', (t) => { const ml = new MockLogger(); ml.level = levels.INFO; diff --git a/test/tap/connect-nolog-test.js b/test/tap/connect-nolog-test.js index 1c81208..efe4cf6 100644 --- a/test/tap/connect-nolog-test.js +++ b/test/tap/connect-nolog-test.js @@ -221,5 +221,52 @@ test('log4js connect logger', (batch) => { t.end(); }); + batch.test('nolog Array', (t) => { + const ml = new MockLogger(); + const cl = clm(ml, { nolog: [/\.gif/, /\.jpe?g/] }); + + t.beforeEach((done) => { ml.messages = []; done(); }); + + t.test('check unmatch url request (png)', (assert) => { + const messages = ml.messages; + const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.png'); // not gif + const res = new MockResponse(200); + cl(req, res, () => { }); + res.end('chunk', 'encoding'); + + assert.equal(messages.length, 1); + assert.ok(levels.INFO.isEqualTo(messages[0].level)); + assert.include(messages[0].message, 'GET'); + assert.include(messages[0].message, 'http://url'); + assert.include(messages[0].message, 'my.remote.addr'); + assert.include(messages[0].message, '200'); + assert.end(); + }); + + t.test('check match url request (gif)', (assert) => { + const messages = ml.messages; + const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.gif'); // gif + const res = new MockResponse(200); + cl(req, res, () => {}); + res.end('chunk', 'encoding'); + + assert.equal(messages.length, 0); + assert.end(); + }); + + t.test('check match url request (jpeg)', (assert) => { + const messages = ml.messages; + const req = new MockRequest('my.remote.addr', 'GET', 'http://url/hoge.jpeg'); // gif + const res = new MockResponse(200); + cl(req, res, () => {}); + res.end('chunk', 'encoding'); + + assert.equal(messages.length, 0); + assert.end(); + }); + + t.end(); + }); + batch.end(); });