apply serializers to args once before asObject or transmit (#1971)

Author: emmyakin

browser.js

@@ -302,7 +302,7 @@ function createWrap (self, opts, rootLogger, level) {
       const proto = (Object.getPrototypeOf && Object.getPrototypeOf(this) === _console) ? _console : this
       for (var i = 0; i < args.length; i++) args[i] = arguments[i]
 
-      if (opts.serialize && !opts.asObject) {
+      if (opts.serialize && !opts.transmit) {
         applySerializers(args, this._serialize, this.serializers, this._stdErrSerialize)
       }
       if (opts.asObject || opts.formatters) {
@@ -330,17 +330,22 @@ function createWrap (self, opts, rootLogger, level) {
 
 function asObject (logger, level, args, ts, formatters = {}) {
   const {
-    level: levelFormatter = () => logger.levels.values[level],
+    level: levelFormatter,
     log: logObjectFormatter = (obj) => obj
   } = formatters
-  if (logger._serialize) applySerializers(args, logger._serialize, logger.serializers, logger._stdErrSerialize)
   const argsCloned = args.slice()
   let msg = argsCloned[0]
   const logObject = {}
   if (ts) {
     logObject.time = ts
   }
-  logObject.level = levelFormatter(level, logger.levels.values[level])
+
+  if (levelFormatter) {
+    const formattedLevel = levelFormatter(level, logger.levels.values[level])
+    Object.assign(logObject, formattedLevel)
+  } else {
+    logObject.level = logger.levels.values[level]
+  }
 
   let lvl = (logger._childLevel | 0) + 1
   if (lvl < 1) lvl = 1
@@ -361,9 +366,9 @@ function applySerializers (args, serialize, serializers, stdErrSerialize) {
   for (const i in args) {
     if (stdErrSerialize && args[i] instanceof Error) {
       args[i] = pino.stdSerializers.err(args[i])
-    } else if (typeof args[i] === 'object' && !Array.isArray(args[i])) {
+    } else if (typeof args[i] === 'object' && !Array.isArray(args[i]) && serialize) {
       for (const k in args[i]) {
-        if (serialize && serialize.indexOf(k) > -1 && k in serializers) {
+        if (serialize.indexOf(k) > -1 && k in serializers) {
           args[i][k] = serializers[k](args[i][k])
         }
       }
@@ -385,6 +390,7 @@ function transmit (logger, opts, args) {
     logger.serializers,
     logger._stdErrSerialize === undefined ? true : logger._stdErrSerialize
   )
+
   logger._logEvent.ts = ts
   logger._logEvent.messages = args.filter(function (arg) {
     // bindings can only be objects, so reference equality check via indexOf is fine

docs/browser.md

@@ -144,7 +144,7 @@ Unlike server pino the serializers apply to every object passed to the logger me
 if the `asObject` option is `true`, this results in the serializers applying to the
 first object (as in server pino).
 
-For more info on serializers see https://github.com/pinojs/pino/blob/master/docs/api.md#parameters.
+For more info on serializers see https://github.com/pinojs/pino/blob/master/docs/api.md#mergingobject.
 
 ### `transmit` (Object)
 

test/browser-serializers.test.js

@@ -284,7 +284,6 @@ test('children inherit parent serializers', ({ end, is }) => {
 
 test('children serializers get called', ({ end, is }) => {
   const parent = pino({
-    test: 'this',
     browser: {
       serialize: true,
       write (o) {
@@ -301,7 +300,6 @@ test('children serializers get called', ({ end, is }) => {
 
 test('children serializers get called when inherited from parent', ({ end, is }) => {
   const parent = pino({
-    test: 'this',
     serializers: parentSerializers,
     browser: {
       serialize: true,

test/browser.test.js

@@ -166,7 +166,7 @@ test('opts.browser.asObject logs pino-like object to console', ({ end, ok, is })
   end()
 })
 
-test('opts.browser.formatters logs pino-like object to console', ({ end, ok, is }) => {
+test('opts.browser.formatters (level) logs pino-like object to console', ({ end, ok, is }) => {
   const info = console.info
   console.info = function (o) {
     is(o.level, 30)
@@ -189,7 +189,7 @@ test('opts.browser.formatters logs pino-like object to console', ({ end, ok, is
   end()
 })
 
-test('opts.browser.formatters logs pino-like object to console', ({ end, ok, is }) => {
+test('opts.browser.formatters (log) logs pino-like object to console', ({ end, ok, is }) => {
   const info = console.info
   console.info = function (o) {
     is(o.level, 30)
@@ -213,40 +213,66 @@ test('opts.browser.formatters logs pino-like object to console', ({ end, ok, is
   end()
 })
 
-test('opts.browser.write func log single string', ({ end, ok, is }) => {
-  const instance = pino({
+test('opts.browser.serialize and opts.browser.transmit only serializes log data once', ({ end, ok, is }) => {
+  const instance = require('../browser')({
+    serializers: {
+      extras (data) {
+        return { serializedExtras: data }
+      }
+    },
     browser: {
+      serialize: ['extras'],
+      transmit: {
+        level: 'info',
+        send (level, o) {
+          is(o.messages[0].extras.serializedExtras, 'world')
+        }
+      }
+    }
+  })
+
+  instance.info({ extras: 'world' }, 'test')
+  end()
+})
+
+test('opts.browser.serialize and opts.asObject only serializes log data once', ({ end, ok, is }) => {
+  const instance = require('../browser')({
+    serializers: {
+      extras (data) {
+        return { serializedExtras: data }
+      }
+    },
+    browser: {
+      serialize: ['extras'],
+      asObject: true,
       write: function (o) {
-        is(o.level, 30)
-        is(o.msg, 'test')
-        ok(o.time)
+        is(o.extras.serializedExtras, 'world')
       }
     }
   })
-  instance.info('test')
 
+  instance.info({ extras: 'world' }, 'test')
   end()
 })
 
-test('opts.browser.write func string joining', ({ end, ok, is }) => {
+test('opts.browser.write func log single string', ({ end, ok, is }) => {
   const instance = pino({
     browser: {
       write: function (o) {
         is(o.level, 30)
-        is(o.msg, 'test test2 test3')
+        is(o.msg, 'test')
         ok(o.time)
       }
     }
   })
-  instance.info('test %s %s', 'test2', 'test3')
+  instance.info('test')
 
   end()
 })
 
-test('opts.browser.write func string joining when asObject is true', ({ end, ok, is }) => {
+test('opts.browser.write func string joining', ({ end, ok, is }) => {
   const instance = pino({
     browser: {
-      asObject: true,
       write: function (o) {
         is(o.level, 30)
         is(o.msg, 'test test2 test3')