pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.21k stars 875 forks source link

Logs are not printing on hard exit #1183

Closed jsumners closed 3 years ago

jsumners commented 3 years ago

I'm not sure there is anything we can do about this except document it?

index.js:

"use strict";

const pino = require("pino");
const transport = pino.transport({
  targets: [
    { target: "./stderr.mjs", level: "debug" },
    { target: "pino/file", level: "info", options: { destination: 1 } },
  ],
});
const log = pino(transport);

log.error("boom");

// With the next line uncommented, no logs will be shown.
process.exit(1);

stderr.mjs:

import { Writable } from "stream";
export default (options) => {
  const myTransportStream = new Writable({
    write(chunk, enc, cb) {
      console.error(chunk.toString());
      cb();
    },
  });
  return myTransportStream;
};
mcollina commented 3 years ago

The reason why this is not working is because console.log() (and friends) in workers send the message to the main thread, which has its event loop shut down already because of process.exit(1).

Just use pino.destination() and it would all work smoothly. In other terms, we should just document this.

jsumners commented 3 years ago

If I modify stderr.mjs to the following, I still see the same issue:

import { Writable } from "stream";
import pino from "pino";

const destination = pino.destination(2);

export default (options) => {
  const myTransportStream = new Writable({
    write(chunk, enc, cb) {
      destination.write(chunk.toString());
      cb();
    },
  });
  return myTransportStream;
};

Are we saying that we simply cannot support a custom transport with hard exits?

mcollina commented 3 years ago

What you are asking is an edge case that we are not handling atm. Essentially we should need at least one event loop spin to start the worker.

"use strict";

const pino = require(".");
const transport = pino.transport({
  target: 'pino/file',
  options: {
    destination: 1
  }
});
const log = pino(transport);

log.info("start");

transport.on('ready', function () {
  log.error("boom");
  // With the next line uncommented, no logs will be shown.
  process.exit(1);
})

However this is not working either, so that's a regression.

mcollina commented 3 years ago

I've found the regression for the latest case. The main one reported (no event loop spin) might require a bit more work however it's totally doable.

jsumners commented 3 years ago

I thought we had implemented something around this. But I wasn't having any luck searching through the issues.

mcollina commented 3 years ago

PR incoming for the latter case https://github.com/pinojs/pino/issues/1183#issuecomment-950940938

github-actions[bot] commented 2 years ago

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.