Support synchronous logging for proper exit handling

[omidkrad]

Exit logging does not work on pino-seq which is crucial for production code. This causes important error logs to get lost on unhandled exceptions. When using exit handlers I get the following error:

Error: final requires a stream that has a flushSync method, such as pino.destination and pino.extreme

These issues https://github.com/pinojs/pino-pretty/issues/37 and https://github.com/pinojs/pino/issues/542 seem to be related. We need seq-logging to support synchronous requests for this to work so we can call it from flushSync implementation in pino-seq and probably other implementors.

[nblumhardt]

Thank you for raising this 👍

Do you have any idea which is the preferred Node API for implementing this?

[omidkrad]

I don't think it needs a Node API, but needs to synchronously flush the logs or wait until all async writes are completed before returning.

My workaround right now is to wait a few seconds and hope that all logs are flushed before exiting.

setTimeout(() => {
   process.exit(1);
}, 5000);

[nblumhardt]

Thanks for the follow-up. I think the origin of the sync requirement is because queued work (such as timeouts) won't be completed in certain process exit scenarios. I believe this is where the need for a synchronous request comes into the picture.

[rj-xy]

Perhaps pino-seq should implement something like this: index.ts#L59

When winston.close() is called, the seq-logger (winston-seq) will flush and close, all the while emitting events respectively. The subscribed events are then called (integration.test.ts#L24) and the async Test can then end.

winstonjs only provides a synchronous close method, so I've had to emit events so that I know the logger has been flushed and closed, to finalise my integration test.

[nblumhardt]

Hi all! Awaiting the logger's flush() method should now work, and no synchronous APIs should be needed:

await logger.flush();

If you still run into any problems or incompatibilities with this please let me know; we'll close this because the codebase has changed drastically since the original issue report was received, and it's not clear that the same constraints/deficiencies still exist today. Thanks!