fix: prettyPrint options - misleading error message with wrong option…

… value (#1012)

* fix: prettyPrint options - misleading error message with wrong options value

* fix: rewrite throws in tests - remove unnecessary code and make code more readable

lib/tools.js

@@ -184,7 +184,10 @@ function getPrettyStream (opts, prettifier, dest, instance) {
     prettyFactory.asMetaWrapper = prettifierMetaWrapper
     return prettifierMetaWrapper(prettyFactory(opts), dest, opts)
   } catch (e) {
-    throw Error('Missing `pino-pretty` module: `pino-pretty` must be installed separately')
+    if (e.message.startsWith("Cannot find module 'pino-pretty'")) {
+      throw Error('Missing `pino-pretty` module: `pino-pretty` must be installed separately')
+    };
+    throw e
   }
 }
 

test/basic.test.js

@@ -616,19 +616,12 @@ test('fast-safe-stringify must be used when interpolating', async (t) => {
   t.equal(msg, 'test {"a":{"b":{"c":"[Circular]"}}}')
 })
 
-test('throws when setting useOnlyCustomLevels without customLevels', async ({ equal, throws }) => {
+test('throws when setting useOnlyCustomLevels without customLevels', async ({ throws }) => {
   throws(() => {
     pino({
       useOnlyCustomLevels: true
     })
-  })
-  try {
-    pino({
-      useOnlyCustomLevels: true
-    })
-  } catch ({ message }) {
-    equal(message, 'customLevels is required if useOnlyCustomLevels is set true')
-  }
+  }, 'customLevels is required if useOnlyCustomLevels is set true')
 })
 
 test('correctly log Infinity', async (t) => {

test/custom-levels.test.js

@@ -125,7 +125,7 @@ test('customLevels property child bindings does not get logged', async ({ equal
   equal(customLevels, undefined)
 })
 
-test('throws when specifying pre-existing parent labels via child bindings', async ({ equal, throws }) => {
+test('throws when specifying pre-existing parent labels via child bindings', async ({ throws }) => {
   const stream = sink()
   throws(() => pino({
     customLevels: {
@@ -135,24 +135,10 @@ test('throws when specifying pre-existing parent labels via child bindings', asy
     customLevels: {
       foo: 45
     }
-  })
-  )
-  try {
-    pino({
-      customLevels: {
-        foo: 35
-      }
-    }, stream).child({
-      customLevels: {
-        foo: 45
-      }
-    })
-  } catch ({ message }) {
-    equal(message, 'levels cannot be overridden')
-  }
+  }), 'levels cannot be overridden')
 })
 
-test('throws when specifying pre-existing parent values via child bindings', async ({ equal, throws }) => {
+test('throws when specifying pre-existing parent values via child bindings', async ({ throws }) => {
   const stream = sink()
   throws(() => pino({
     customLevels: {
@@ -162,55 +148,23 @@ test('throws when specifying pre-existing parent values via child bindings', asy
     customLevels: {
       bar: 35
     }
-  })
-  )
-  try {
-    pino({
-      customLevels: {
-        foo: 35
-      }
-    }, stream).child({
-      customLevels: {
-        bar: 35
-      }
-    })
-  } catch ({ message }) {
-    equal(message, 'pre-existing level values cannot be used for new levels')
-  }
+  }), 'pre-existing level values cannot be used for new levels')
 })
 
-test('throws when specifying core values via child bindings', async ({ equal, throws }) => {
+test('throws when specifying core values via child bindings', async ({ throws }) => {
   const stream = sink()
   throws(() => pino(stream).child({
     customLevels: {
       foo: 30
     }
-  })
-  )
-  try {
-    pino(stream).child({
-      customLevels: {
-        foo: 30
-      }
-    })
-  } catch ({ message }) {
-    equal(message, 'pre-existing level values cannot be used for new levels')
-  }
+  }), 'pre-existing level values cannot be used for new levels')
 })
 
-test('throws when useOnlyCustomLevels is set true without customLevels', async ({ equal, throws }) => {
+test('throws when useOnlyCustomLevels is set true without customLevels', async ({ throws }) => {
   const stream = sink()
   throws(() => pino({
     useOnlyCustomLevels: true
-  }, stream)
-  )
-  try {
-    pino({
-      useOnlyCustomLevels: true
-    }, stream)
-  } catch ({ message }) {
-    equal(message, 'customLevels is required if useOnlyCustomLevels is set true')
-  }
+  }, stream), 'customLevels is required if useOnlyCustomLevels is set true')
 })
 
 test('custom level on one instance does not affect other instances', async ({ equal }) => {

test/levels.test.js

@@ -378,7 +378,7 @@ test('setting levelKey does not affect labels when told to', async ({ equal }) =
   instance.info('hello world')
 })
 
-test('throws when creating a default label that does not exist in logger levels', async ({ equal, throws }) => {
+test('throws when creating a default label that does not exist in logger levels', async ({ throws }) => {
   const defaultLevel = 'foo'
   throws(() => {
     pino({
@@ -387,17 +387,10 @@ test('throws when creating a default label that does not exist in logger levels'
       },
       level: defaultLevel
     })
-  })
-  try {
-    pino({
-      level: defaultLevel
-    })
-  } catch ({ message }) {
-    equal(message, `default level:${defaultLevel} must be included in custom levels`)
-  }
+  }, `default level:${defaultLevel} must be included in custom levels`)
 })
 
-test('throws when creating a default value that does not exist in logger levels', async ({ equal, throws }) => {
+test('throws when creating a default value that does not exist in logger levels', async ({ throws }) => {
   const defaultLevel = 15
   throws(() => {
     pino({
@@ -406,14 +399,7 @@ test('throws when creating a default value that does not exist in logger levels'
       },
       level: defaultLevel
     })
-  })
-  try {
-    pino({
-      level: defaultLevel
-    })
-  } catch ({ message }) {
-    equal(message, `default level:${defaultLevel} must be included in custom levels`)
-  }
+  }, `default level:${defaultLevel} must be included in custom levels`)
 })
 
 test('throws when creating a default value that does not exist in logger levels', async ({ equal, throws }) => {
@@ -424,17 +410,7 @@ test('throws when creating a default value that does not exist in logger levels'
       },
       useOnlyCustomLevels: true
     })
-  })
-  try {
-    pino({
-      customLevels: {
-        foo: 5
-      },
-      useOnlyCustomLevels: true
-    })
-  } catch ({ message }) {
-    equal(message, 'default level:info must be included in custom levels')
-  }
+  }, 'default level:info must be included in custom levels')
 })
 
 test('passes when creating a default value that exists in logger levels', async ({ equal, throws }) => {

test/pretty.test.js

@@ -50,22 +50,22 @@ test('does not throw error when enabled with stream specified', async ({ doesNot
   doesNotThrow(() => pino({ prettyPrint: true }, process.stdout))
 })
 
-test('throws when prettyPrint is true but pino-pretty module is not installed', async ({ throws, equal }) => {
+test('throws when prettyPrint is true but pino-pretty module is not installed', async ({ throws }) => {
   // pino pretty *is* installed, and probably also cached, so rather than
   // messing with the filesystem the simplest way to generate a not found
   // error is to simulate it:
   const prettyFactory = require('pino-pretty')
   require.cache[require.resolve('pino-pretty')].exports = () => {
     throw Error('Cannot find module \'pino-pretty\'')
   }
-  throws(() => pino({ prettyPrint: true }))
-  try { pino({ prettyPrint: true }) } catch ({ message }) {
-    equal(message, 'Missing `pino-pretty` module: `pino-pretty` must be installed separately')
-  }
-
+  throws(() => pino({ prettyPrint: true }), 'Missing `pino-pretty` module: `pino-pretty` must be installed separately')
   require.cache[require.resolve('pino-pretty')].exports = prettyFactory
 })
 
+test('throws when prettyPrint has invalid options', async ({ throws }) => {
+  throws(() => pino({ prettyPrint: { ignore: ['hostname'] } }), 'opts.ignore.split is not a function')
+})
+
 test('can send pretty print to custom stream', async ({ equal }) => {
   const dest = new Writable({
     objectMode: true,