Repository: pinojs/pino (js)

Issue:
`pino.final` without a handler and `stream.flushSync()`
I discovered that pino.final can be used without a handler, which I find very handy. Example:

```js
const handler = (err) => {
  const finalLogger = pino.final(logger);
  
  if (err) finalLogger(err);
  process.exit(err ? 1 : 0);
};
```

But reading the code I found something that might be a bug and perhaps someone here can confirm it.

https://github.com/pinojs/pino/blob/c57c25855d89a2c481914672bbdeebe7ec3087c7/lib/tools.js#L463-L478

`stream.flushSync()` is only called before the handler is executed or after `finalLogger` is used. In the example above there is no handler and `finalLogger` is only called if there is an error, so when the process exits normally there is no call to `stream.flushSync()`.

Perhaps the implemention when there is no handler should be this

```js
  if (!hasHandler) {
    try {
      stream.flushSync()
    } catch (e) {
      // it's too late to wait for the stream to be ready
      // because this is a final tick scenario.
      // in practice there shouldn't be a situation where it isn't
      // however, swallow the error just in case (and for easier testing)
    }
    return finalLogger
  }
```

Pull request description:
fix: make pino.final sync flushes when no handler provided (#1126)
Fixes #1126 

Patch:
diff --git a/lib/tools.js b/lib/tools.js
index 63bb746..7a9dc80 100644
--- a/lib/tools.js
+++ b/lib/tools.js
@@ -461,6 +461,14 @@ function final (logger, handler) {
   })
 
   if (!hasHandler) {
+    try {
+      stream.flushSync()
+    } catch {
+      // it's too late to wait for the stream to be ready
+      // because this is a final tick scenario.
+      // in practice there shouldn't be a situation where it isn't
+      // however, swallow the error just in case (and for easier testing)
+    }
     return finalLogger
   }
 


Test patch:
diff --git a/test/final.test.js b/test/final.test.js
index 300f59d..4828920 100644
--- a/test/final.test.js
+++ b/test/final.test.js
@@ -72,6 +72,32 @@ test('listener function immediately sync flushes when fired (sync true)', async
   if (passed === false) fail('flushSync not called')
 })
 
+test('immediately sync flushes when no handler is provided (sync false)', async ({ pass, fail }) => {
+  const dest = pino.destination({ dest: getPathToNull(), sync: false })
+  let passed = false
+  dest.flushSync = () => {
+    passed = true
+    pass('flushSync called')
+    dest.flushSync = () => {}
+  }
+  pino.final(pino(dest))
+  await sleep(10)
+  if (passed === false) fail('flushSync not called')
+})
+
+test('immediately sync flushes when no handler is provided (sync true)', async ({ pass, fail }) => {
+  const dest = pino.destination({ dest: getPathToNull(), sync: true })
+  let passed = false
+  dest.flushSync = () => {
+    passed = true
+    pass('flushSync called')
+    dest.flushSync = () => {}
+  }
+  pino.final(pino(dest))
+  await sleep(10)
+  if (passed === false) fail('flushSync not called')
+})
+
 test('swallows the non-ready error', async ({ doesNotThrow }) => {
   const dest = pino.destination({ dest: getPathToNull(), sync: false })
   doesNotThrow(() => {