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

feat: log even on closed connections #117

Closed

Conversation

dalexanco
Copy link

Hi, this PR change the event used by pino-http on response from 'finish' to 'close'.
It let the opportunity to log request which does not have open socket anymore.

I add a new option "onlyLogFinishedRequest" set by default to true (to keep existing behavior).

More information : #116

this.removeListener('finish', onResFinished)
>>>>>>> Stashed changes
Copy link
Member

Choose a reason for hiding this comment

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

It seems you have made some git mistakes :). Would you mind to fix them?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@dalexanco feel free to send a fresh PR if you'd like.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mcollina: #123
@dalexanco: I really wanted to get this merged upstream, so, I checked out your branch and committed the fix with you as the co-author; all your other commits are left intact. Thanks for the fix!

Copy link
Author

Choose a reason for hiding this comment

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

my bad, I have been away for a while.
Thank you for the 2nd PR !
Really smart to allow the feature only on v12, that was the real reason of my absence on this PR : I fail to make it work on v10 😅

@mcollina
Copy link
Member

mcollina commented Jul 4, 2022

closed for no activity

@mcollina mcollina closed this Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants