Makes final warn that is not supported instead of throwing

[mcollina]

Fixes pinojs/pino-pretty#37

[jsumners]

LGTM

[davidmarkclements]

I've read up on the issue - not sure I agree that warning is better than throwing.

Throwing is the right thing because you shouldn't use pino.final at all with streams

I'd rather pino-pretty catches the error and then prints a warning itself

[mcollina]

I've read up on the issue - not sure I agree that warning is better than throwing.

I think it is. The following currently won't work, and a user would have to add another if for process.env.NODE_ENV:

const pino = require('pino')

const logger = pino({
  prettyPrint: process.env.NODE_ENV === 'development'
})

const run = async () => {
  process.on('SIGTERM', pino.final(logger, (err, finalLogger) => {
    finalLogger.info('Graceful shutdown 1')
    finalLogger.info('Graceful shutdown 2')
    process.exit(0)
  }))

  await delay(30)
}

const delay = (secs) => {
  return new Promise((resolve, reject) => {
    setTimeout(() => resolve(), secs * 1000)
  })
}

run().catch(err => {
  logger.error({ err })
})

Throwing is the right thing because you shouldn't use pino.final at all with streams

I'd rather pino-pretty catches the error and then prints a warning itself

You cannot catch that error, as pino-pretty is not involved at all in the check. Another option is to replace the instanceof check with a typeof stream.flushSync === 'function', and let ducktyping do its magic. The stream we use for pinoPretty: true would have this function which will emit an error when called. Would this be better?

[davidmarkclements]

yeah I think it caters to the two targeted cases -

1. output a warning when using pretty print with pino.final
2. throw when using any other stream with pino.final

[mcollina]

PTAL

[davidmarkclements]

typo: inal -> final

[davidmarkclements]

other than the typo - LGTM

[davidmarkclements]

Lgtm

[github-actions]

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.