Skip to content

Fix connectlogger nolog function#1285

Merged
lamweili merged 2 commits intolog4js-node:masterfrom
eyoboue:fix-connectlogger-function
Jul 8, 2022
Merged

Fix connectlogger nolog function#1285
lamweili merged 2 commits intolog4js-node:masterfrom
eyoboue:fix-connectlogger-function

Conversation

@eyoboue
Copy link
Copy Markdown
Contributor

@eyoboue eyoboue commented Jul 7, 2022

Fix #1284.
The server headers are now avalaible in nolog function.

@lamweili
Copy link
Copy Markdown
Contributor

lamweili commented Jul 7, 2022

Hi, I don't understand how that fixes the issue.
Can you walk me through it?

Also, please add tests. Thanks!

@eyoboue
Copy link
Copy Markdown
Contributor Author

eyoboue commented Jul 7, 2022

I meant server response headers like Content-Type will be undefined in current nolog function.
These headers are avalaible only inside handler function at line 282.
So this never get server response content-type header:

nolog: (req, res) => {
    return /(css|javascript|image|font|svg|png|jpe?g)/i.test(res.getHeader('content-type'));
}

@lamweili
Copy link
Copy Markdown
Contributor

lamweili commented Jul 7, 2022

Ok, can you add tests?

@eyoboue
Copy link
Copy Markdown
Contributor Author

eyoboue commented Jul 7, 2022

Ok, Roger that

@eyoboue

This comment was marked as off-topic.

@eyoboue
Copy link
Copy Markdown
Contributor Author

eyoboue commented Jul 8, 2022

Ok, can you add tests?

Test coverage done.

@lamweili lamweili force-pushed the fix-connectlogger-function branch from 25eacc9 to e71e088 Compare July 8, 2022 13:33
Copy link
Copy Markdown
Contributor

@lamweili lamweili left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@lamweili lamweili added this to the 6.6.1 milestone Jul 8, 2022
@lamweili lamweili added the bug Something isn't working label Jul 8, 2022
@lamweili lamweili merged commit 39218cc into log4js-node:master Jul 8, 2022
@lamweili lamweili changed the title Fix connectlogger function Fix connectlogger nolog function Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

connectLogger with nolog: (req, res) doesn't retrieve server headers

2 participants