Repository: pinojs/pino (js)

Issue:
Add a deprecation warning for pino.final() in Node v14+ 
> Correct! How about we emit the warning if on Node v14+?

Seems acceptable.

_Originally posted by @jsumners in https://github.com/pinojs/pino/issues/1171#issuecomment-947647249_

Pull request:
Deprecation warning for pino.final() in Node v14+
This PR adds 
- Deprecation warning for pino.final() in Node v14+

Closes #1175 

Patch:
diff --git a/docs/transports.md b/docs/transports.md
index 4242bfa..e9adcc0 100644
--- a/docs/transports.md
+++ b/docs/transports.md
@@ -23,17 +23,10 @@ and can be used or configured via the options object passed to `pino` on initial
 A transport is a module that exports a default function which returns a writable stream:
 
 ```js
-import { Writable } from 'stream'
+import { createWriteStream } from 'fs'
+
 export default (options) => {
-  const myTransportStream = new Writable({
-    autoDestroy: true, // Needed for Node v12+
-    write (chunk, enc, cb) {
-    // apply a transform and send to stdout
-    console.log(chunk.toString().toUpperCase())
-    cb()
-    }
-  })
-  return myTransportStream
+  return createWriteStream(options.destination)
 }
 ```
 
@@ -55,7 +48,8 @@ The transport code will be executed in a separate worker thread. The main thread
 will write logs to the worker thread, which will write them to the stream returned
 from the function exported from the transport file/module.
 
-The exported function can also be async. Imagine the following transport:
+The exported function can also be async. If we use an async function we can throw early
+if the transform could not be opened. As an example:
 
 ```js
 import fs from 'fs'
@@ -107,6 +101,7 @@ For more details on `pino.transport` see the [API docs for `pino.transport`][pin
 [pino-transport]: /docs/api.md#pino-transport
 [sca]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API/Structured_clone_algorithm
 
+<a id="writing"></a>
 ### Writing a Transport
 
 The module [pino-abstract-transport](https://github.com/pinojs/pino-abstract-transport) provides
@@ -116,11 +111,27 @@ You can see an example using a async iterator with ESM:
 
 ```js
 import build from 'pino-abstract-transport'
+import SonicBoom from 'sonic-boom'
+import { once } from 'events'
 
 export default async function (opts) {
+  // SonicBoom is necessary to avoid loops with the main thread.
+  // It is the same of pino.destination().
+  const destination = new SonicBoom({ dest: opts.destination || 1, sync: false })
+  await once(destination, 'ready')
+
   return build(async function (source) {
     for await (let obj of source) {
-      console.log(obj)
+      const toDrain = !destination.write(obj.message.toUpperCase() + '\n')
+      // This block will handle backpressure
+      if (toDrain) {
+        await once(destination, 'drain')
+      }
+    }
+  }, {
+    async close (err) {
+      destination.end()
+      await once(destination, 'close')
     }
   })
 }
@@ -132,12 +143,17 @@ or using Node.js streams and CommonJS:
 'use strict'
 
 const build = require('pino-abstract-transport')
+const SonicBoom = require('sonic-boom')
 
 module.exports = function (opts) {
+  const destination = new SonicBoom({ dest: opts.destination || 1, sync: false })
   return build(function (source) {
-    source.on('data', function (obj) {
-      console.log(obj)
-    })
+    source.pipe(destination)
+  }, {
+    close (err, cb) {
+      destination.end()
+      destination.on('close', cb.bind(null, err))
+    }
   })
 }
 ```
@@ -146,6 +162,9 @@ module.exports = function (opts) {
 
 To consume async iterators in batches, consider using the [hwp](https://github.com/mcollina/hwp) library.
 
+The `close()` function is needed to make sure that the stream is closed and flushed when its
+callback is called or the returned promise resolved. Otherwise log lines will be lost.
+
 ### Creating a transport pipeline
 
 As an example, the following transport returns a `Transform` stream:
diff --git a/lib/deprecations.js b/lib/deprecations.js
index a6f4d78..d6c6224 100644
--- a/lib/deprecations.js
+++ b/lib/deprecations.js
@@ -6,3 +6,5 @@ module.exports = warning
 const warnName = 'PinoWarning'
 
 warning.create(warnName, 'PINODEP008', 'prettyPrint is deprecated, look at https://github.com/pinojs/pino-pretty for alternatives.')
+
+warning.create(warnName, 'PINODEP009', 'The use of pino.final is discouraged in Node.js v14+ and not required. It will be removed in the next major version')
diff --git a/lib/tools.js b/lib/tools.js
index f32fe53..10a48bb 100644
--- a/lib/tools.js
+++ b/lib/tools.js
@@ -437,6 +437,9 @@ function createArgsNormalizer (defaultOptions) {
 }
 
 function final (logger, handler) {
+  const major = Number(process.versions.node.split('.')[0])
+  if (major >= 14) warning.emit('PINODEP009')
+
   if (typeof logger === 'undefined' || typeof logger.child !== 'function') {
     throw Error('expected a pino logger instance')
   }
diff --git a/package.json b/package.json
index 42170f5..b470926 100644
--- a/package.json
+++ b/package.json
@@ -111,7 +111,7 @@
     "quick-format-unescaped": "^4.0.3",
     "safe-stable-stringify": "^2.1.0",
     "sonic-boom": "^2.2.1",
-    "thread-stream": "^0.11.1"
+    "thread-stream": "^0.12.0"
   },
   "tsd": {
     "directory": "test/types"


Test patch:
diff --git a/test/final.test.js b/test/final.test.js
index 4828920..14faf32 100644
--- a/test/final.test.js
+++ b/test/final.test.js
@@ -8,6 +8,29 @@ const { sleep, getPathToNull } = require('./helper')
 // will be emitted. Let's raise this so we do not scare everybody.
 process.setMaxListeners(100)
 
+test('should show warning for pino.final on node 14+', ({ equal, end, plan }) => {
+  const major = Number(process.versions.node.split('.')[0])
+  if (major < 14) return end()
+
+  plan(1)
+  const dest = pino.destination({ dest: getPathToNull(), sync: false })
+  dest.flushSync = () => {}
+  const instance = pino(dest)
+
+  pino.final(instance, (_, finalLogger) => {
+    finalLogger.info('hello')
+  })()
+
+  function onWarning (warning) {
+    equal(warning.code, 'PINODEP009')
+    end()
+  }
+
+  process.once('warning', onWarning)
+
+  instance.info('hello')
+})
+
 test('replaces onTerminated option', async ({ throws }) => {
   throws(() => {
     pino({