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

Fix connectlogger nolog function #1285

Merged
merged 2 commits into from Jul 8, 2022
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
15 changes: 11 additions & 4 deletions lib/connect-logger.js
Expand Up @@ -251,12 +251,10 @@ module.exports = function getLogger(logger4js, options) {

return (req, res, next) => {
// mount safety
if (req._logging) return next();
if (req._logging !== undefined) return next();

// nologs
if (typeof options.nolog === 'function') {
if (options.nolog(req, res) === true) return next();
} else {
if (typeof options.nolog !== 'function') {
const nolog = createNoLogCondition(options.nolog);
if (nolog && nolog.test(req.originalUrl)) return next();
}
Expand Down Expand Up @@ -284,6 +282,15 @@ module.exports = function getLogger(logger4js, options) {
return;
}
finished = true;

// nologs
if (typeof options.nolog === 'function') {
if (options.nolog(req, res) === true) {
req._logging = false;
return;
}
}

res.responseTime = new Date() - start;
// status code response level handling
if (res.statusCode && options.level === 'auto') {
Expand Down
4 changes: 3 additions & 1 deletion test/tap/configuration-validation-test.js
Expand Up @@ -274,7 +274,9 @@ test('log4js configuration validation', (batch) => {
})
)
);
t.end();
log4js.shutdown(() => {
t.end();
});
});

batch.test('should load appenders from core first', (t) => {
Expand Down
26 changes: 25 additions & 1 deletion test/tap/connect-nolog-test.js
Expand Up @@ -348,7 +348,10 @@ test('log4js connect logger', (batch) => {

batch.test('nolog function', (t) => {
const ml = new MockLogger();
const cl = clm(ml, { nolog: (_req, res) => res.statusCode < 400 });
const cl = clm(ml, {
nolog: (_req, res) =>
res.getHeader('content-type') === 'image/png' || res.statusCode < 400,
});

t.beforeEach(() => {
ml.messages = [];
Expand Down Expand Up @@ -381,6 +384,27 @@ test('log4js connect logger', (batch) => {
assert.end();
});

t.test(
'check match function server response content-type header',
(assert) => {
const { messages } = ml;
const req = new MockRequest(
'my.remote.addr',
'GET',
'http://url/nolog'
);
const res = new MockResponse(500);
res.on('finish', () => {
res.setHeader('content-type', 'image/png');
});
cl(req, res, () => {});
res.end('chunk', 'encoding');

assert.equal(messages.length, 0);
assert.end();
}
);

t.end();
});

Expand Down