-
Notifications
You must be signed in to change notification settings - Fork 885
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
Automatically flush the logs on exit in Node v14+ #1091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@@ -37,7 +37,9 @@ test('pino with no args log everything when calling process.exit(0)', async ({ n | |||
not(actual.match(/world/), null) | |||
}) | |||
|
|||
test('sync false does not log everything when calling process.exit(0)', async ({ equal }) => { | |||
const hasWeak = !!global.WeakRef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I despise double inverses more than coercing checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh 🤷 all forms of boolean conversion are a bit shit. Boolean(WeakRef)
is where you call a class without new
to cast it. ==
is not good for obvious reasons, !~~
is too esoteric. I suppose you could use 'WeakRef' in global
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outstanding, I love this. CI has an issue, but preapproving
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR removes the main developer experience hurdle in using asynchronous logging: the use of
pino.final()
.Thanks to
on-exit-leak-free
we can automatically flush on exit without leaking event emitter listeners (in most cases).This uses the same mechanism I experimented with for
pino.transport()
.