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

Pino will not print logs when the event loop is blocked, but console.log will #1801

Closed
vedantroy opened this issue Sep 8, 2023 · 4 comments

Comments

@vedantroy
Copy link

I'm running into an odd error where Pino is not printing logs unless an exception is thrown.

Here's my config:

const opts: PinoOpts = {
  redact: {
    // these just cause clutter
    paths: ["pid", "hostname"],
    remove: true,
  },
  transport: {
    targets: [],
  },
}
opts.transport.targets.push({
  level: "trace",
  target: "pino-pretty",
  options: {
  },
})

const logger = pino(opts)

console.log("BEFORE ...")
logger.info("SOME LOG MESSAGE ....")
console.log("AFTER ...")

// if I comment the below line, I don't see any logs
throw new Error("hi")

What could be be going on? Is there some sort of buffering?

@vedantroy
Copy link
Author

vedantroy commented Sep 8, 2023

Ok, I figured out the issue. When the event-loop is almost blocked because there's an infinite while-loop, pino will not print anything. The problem is -- this is quite bad, because it is precisely when the event loop is blocked that I don't want my logs to go silent.

I'd rather sacrifice lots of performance, then have pino not prevent logs when the event loop is blocked. Example: (will add)

@vedantroy vedantroy changed the title Pino not printing logs unless an exception is thrown? Pino will not print logs when the event loop is blocked, but console.log will Sep 8, 2023
@mcollina
Copy link
Member

mcollina commented Sep 9, 2023

Regarding console.log: that is true only if you are logging to a TTY. If you are logging to another process, or a file, it would be asynchronous as well, with exactly the same behavior (at some point the process will stop logging and will crash with out of memory).

Regarding transports, I actually think we can support this behavior by allowing users to set sync: true in ThreadStream constructor. I actually added that with this specific purpose long ago but in the end I didn't feel the need. If you want to send a PR, it would be awesome.

This is generically a tradeoff between various concerns and it really depends on your application. The defaults here are generically the best tradeoff we could came up with.
You can read more about this topic in https://adventures.nodeland.dev/archive/the-dangers-of-logging-and-other-adventures/.

@karankraina
Copy link
Contributor

@mcollina Since the PR is now merged, should this be closed ?

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants