`.flush()` doesn't work when pretty-print is enabled

[roim]

TSIA

https://github.com/roim/pino-pretty.async-flush

[roim]

Funny enough, when you try to use final you get:

pino.final with prettyPrint does not support flushing

The limitation isn't just for final, yet regular loggers don't have a warning.

[mcollina]

Unfortunately so. We do not think it is needed either as prettyPrint is just something for development - so we did not implement it. If you would like to work on it, I'll be happy to review.

Note that https://github.com/pinojs/pino/pull/1003 will make all sort of things possible.

[roim]

I agree it's not needed (can't think of a scenario where you'd need pretty print and async together). This is more of an usability issue--eg it took me maybe 30 min to realize what was wrong in my pino integration.

Do you guys have a stance on how to mitigate these usability pain points? I'd be happy to either log a warning on .flush calls with async mode enabled (assume the perf hit there is negligible since .flush is already intensive), or to add some doc hints, etc

[mcollina]

I'm ok with both a doc approach and a noisy one. @jsumners?

[jsumners]

I am okay with both.

Also, I do not know what "TSIA" means.

[micalevisk]

I do not know what "TSIA" means.

Title Says It All :smile:

[satya-nutella]

@mcollina @jsumners I'd love to work on this. This is going to be my first contribution to pinojs. Any pointers on what would be a good start?

[mcollina]

Update the doc of pino.final() telling that prettyPrint would not work. Also add it to the docs of prettyPrint.

[davorpa]

Hey @mcollina @roim 👋

I'm trying to do my first contribution with this. 🤗 After reading how final and asynchronous logging works, I think showcase could be already covered.

📑 pino.final() notes: https://github.com/pinojs/pino/blob/01a8633d7ff1b59e9fa47cc47e34421053fea10e/docs/api.md#L1016-L1027

📑 Asynchronous logging notes: https://github.com/pinojs/pino/blob/01a8633d7ff1b59e9fa47cc47e34421053fea10e/docs/asynchronous.md#L74-L108

📑 pino({options.prettyPrint}) notes: https://github.com/pinojs/pino/blob/01a8633d7ff1b59e9fa47cc47e34421053fea10e/docs/api.md#L345-L363 📑 options.suppressFlushSyncWarning notes: https://github.com/pinojs/pino/blob/01a8633d7ff1b59e9fa47cc47e34421053fea10e/docs/pretty.md#L85-L100 ↘ suppressFlushSyncWarning = true: https://github.com/pinojs/pino/blob/01a8633d7ff1b59e9fa47cc47e34421053fea10e/lib/tools.js#L234-L247

Let me know if I'm wrong 😉.

Last note about suppressFlushSyncWarning seems to be the noisy approach 🤔. It's warned only on first flush. Then, how to proceed? 💡Maybe mention in final() and pretty printers to know how to disable it?

BR

[mcollina]

Doc would be good, yes! Would you like to send a PR?

[davorpa]

I'll try @mcollina.

Let me see how I can write according to the rest of the documentation