Coerce string integer destinations to file descriptors (#1180)

Author: jsumners

docs/api.md

@@ -422,7 +422,7 @@ Default: `pino.destination(1)` (STDOUT)
 The `destination` parameter, at a minimum must be an object with a `write` method.
 An ordinary Node.js `stream` can be passed as the destination (such as the result
 of `fs.createWriteStream`) but for peak log writing performance it is strongly
-recommended to use `pino.destination` to create the destination stream. 
+recommended to use `pino.destination` to create the destination stream.
 Note that the `destination` parameter can be the result of `pino.transport()`.
 
 ```js
@@ -449,6 +449,10 @@ However, there are some special instances where `pino.destination` is not used a
 
 In these cases `process.stdout` is used instead.
 
+Note: If the parameter is a string integer, e.g. `'1'`, it will be coerced to
+a number and used as a file descriptor. If this is not desired, provide a full
+path, e.g. `/tmp/1`.
+
 * See [`pino.destination`](#pino-destination)
 
 <a id="metadata"></a>
@@ -1010,7 +1014,7 @@ is flushed and all data synced before the process exits.
 Note that calling `process.exit()` on the main thread will stop the event loop on the main thread from turning. As a result,
 using `console.log` and `process.stdout` after the main thread called `process.exit()` will not produce any output.
 
-If you are embedding/integrating pino within your framework, you will need to make pino aware of the script that is calling it, 
+If you are embedding/integrating pino within your framework, you will need to make pino aware of the script that is calling it,
 like so:
 
 ```js

docs/transports.md

@@ -262,7 +262,7 @@ const transport = pino.transport({
 pino(transport)
 ```
 
-The `options.destination` property may also be a number to represent a file descriptor. Typically this would be `1` to write to STDOUT or `2` to write to STDERR. If `options.destination` is not set, it defaults to `1` which means logs will be written to STDOUT.
+The `options.destination` property may also be a number to represent a filedescriptor. Typically this would be `1` to write to STDOUT or `2` to write to STDERR. If `options.destination` is not set, it defaults to `1` which means logs will be written to STDOUT. If `options.destination` is a string integer, e.g. `'1'`, it will be coerced to a number and used as a file descriptor. If this is not desired, provide a full path, e.g. `/tmp/1`.
 
 The difference between using the `pino/file` transport builtin and using `pino.destination` is that `pino.destination` runs in the main thread, whereas `pino/file` sets up `pino.destination` in a worker thread.
 

lib/tools.js

@@ -520,6 +520,22 @@ function setMetadataProps (dest, that) {
   }
 }
 
+/**
+ * Convert a string integer file descriptor to a proper native integer
+ * file descriptor.
+ *
+ * @param {string} destination The file descriptor string to attempt to convert.
+ *
+ * @returns {Number}
+ */
+function normalizeDestFileDescriptor (destination) {
+  const fd = Number(destination)
+  if (typeof destination === 'string' && Number.isFinite(fd)) {
+    return fd
+  }
+  return destination
+}
+
 module.exports = {
   noop,
   buildSafeSonicBoom,
@@ -530,5 +546,6 @@ module.exports = {
   createArgsNormalizer,
   final,
   stringify,
-  buildFormatters
+  buildFormatters,
+  normalizeDestFileDescriptor
 }

pino.js

@@ -15,8 +15,9 @@ const {
   final,
   buildSafeSonicBoom,
   buildFormatters,
-  noop,
-  stringify
+  stringify,
+  normalizeDestFileDescriptor,
+  noop
 } = require('./lib/tools')
 const { version } = require('./lib/meta')
 const {
@@ -191,10 +192,10 @@ module.exports = pino
 
 module.exports.destination = (dest = process.stdout.fd) => {
   if (typeof dest === 'object') {
-    dest.dest = dest.dest || process.stdout.fd
+    dest.dest = normalizeDestFileDescriptor(dest.dest || process.stdout.fd)
     return buildSafeSonicBoom(dest)
   } else {
-    return buildSafeSonicBoom({ dest, minLength: 0, sync: true })
+    return buildSafeSonicBoom({ dest: normalizeDestFileDescriptor(dest), minLength: 0, sync: true })
   }
 }
 

test/fixtures/transport-string-stdout.js

@@ -0,0 +1,9 @@
+'use strict'
+
+const pino = require('../..')
+const transport = pino.transport({
+  target: 'pino/file',
+  options: { destination: '1' }
+})
+const logger = pino(transport)
+logger.info('Hello')

test/transport/core.test.js

@@ -377,6 +377,18 @@ test('log and exit before ready', async ({ not }) => {
   not(strip(actual).match(/Hello/), null)
 })
 
+test('string integer destination', async ({ not }) => {
+  let actual = ''
+  const child = execa(process.argv[0], [join(__dirname, '..', 'fixtures', 'transport-string-stdout.js')])
+
+  child.stdout.pipe(writer((s, enc, cb) => {
+    actual += s
+    cb()
+  }))
+  await once(child, 'close')
+  not(strip(actual).match(/Hello/), null)
+})
+
 test('pino transport options with target', async ({ teardown, same }) => {
   const destination = join(
     os.tmpdir(),