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

Conversation

eyoboue
Copy link
Contributor

@eyoboue eyoboue commented Jul 7, 2022

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

@lamweili
Copy link
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
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
Contributor

lamweili commented Jul 7, 2022

Ok, can you add tests?

@eyoboue
Copy link
Contributor Author

eyoboue commented Jul 7, 2022

Ok, Roger that

@eyoboue

This comment was marked as off-topic.

@eyoboue
Copy link
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
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