Repository: pinojs/pino (js)

Issue:
Browser API: logEvent.messages cuts off first/main message
Thank you for building this universal solution. Very nice.

However I'm running into an issue when logging in a browser. The goal is to use the `send` method to post the message to a backend service.

Here is my code:

```js
const logger = require('pino')({
  browser: {
    asObject: true,
    transmit: {
      level: 'warn',
      send(level, logEvent) {
        console.log('the event', level, logEvent.messages);
      }
    }
  }
});

logger.info('hello world');
logger.warn('look out world');
logger.error('goodbye world', 'something else');
```

...and the pino logger output when run both client and server-side (ignoring `pid`/`hostname`/`v`):

```bash
{"level":30,"time":1547591483164,"msg":"hello world"}
{"level":40,"time":1547591483164,"msg":"look out world"}
{"level":50,"time":1547591483164,"msg":"goodbye world something else"}
```
Great!

Also, the `send` function is correctly only running for `warn` and `error` (given the minimum level in the `transmit` configuration). However, the `send` method is not holding the right messages. Here is the `console.log` output:

```
the event warn []
the event error ["something else"]
```

This gives me the idea that the first argument passed into `logger.[verb]()` is ignored. However, the [documentation](https://getpino.io/#/docs/browser?id=transmit-object) seems to say otherwise.

Am I missing something? As this seems like a relatively straightforward implementation. Is there a better way to send the log message to a backend service? We don't necessarily need it as an array but it seems like the correct way to retrieve and send it. We may need to end up making a custom transport but wanted to be sure I wasn't missing anything simple.

Using webpack with default configuration. Node 8.9.x, Chrome latest. I can spin up a demo repo if needed.

Thanks for your time.

Pull request:
fix: browser-transmit messages array when asObject is true
Fixes #582 

When `asObject` is `true`, and logging with _just_ strings, the `messages` array does not contain all of the messages.

`args.slice()` creates a clone of `args`, but it was only happening in the case of log messages with objects.

This fixes the issue and expands the test case to include all 4 permutations.



Patch:
diff --git a/browser.js b/browser.js
index 42973fc..a6384c6 100644
--- a/browser.js
+++ b/browser.js
@@ -203,18 +203,18 @@ function wrap (opts, logger, level) {
 
 function asObject (logger, level, args, ts) {
   if (logger._serialize) applySerializers(args, logger._serialize, logger.serializers, logger._stdErrSerialize)
-  var msg = args[0]
+  var argsCloned = args.slice()
+  var msg = argsCloned[0]
   var o = { time: ts, level: pino.levels.values[level] }
   var lvl = (logger._childLevel | 0) + 1
   if (lvl < 1) lvl = 1
   // deliberate, catching objects, arrays
   if (msg !== null && typeof msg === 'object') {
-    args = args.slice()
-    while (lvl-- && typeof args[0] === 'object') {
-      Object.assign(o, args.shift())
+    while (lvl-- && typeof argsCloned[0] === 'object') {
+      Object.assign(o, argsCloned.shift())
     }
-    msg = args.length ? format(args.shift(), args) : undefined
-  } else if (typeof msg === 'string') msg = format(args.shift(), args)
+    msg = argsCloned.length ? format(argsCloned.shift(), argsCloned) : undefined
+  } else if (typeof msg === 'string') msg = format(argsCloned.shift(), argsCloned)
   if (msg !== undefined) o.msg = msg
   return o
 }


Test patch:
diff --git a/test/browser-transmit.test.js b/test/browser-transmit.test.js
index 801d462..8490276 100644
--- a/test/browser-transmit.test.js
+++ b/test/browser-transmit.test.js
@@ -49,9 +49,65 @@ test('passes send function the logged level', ({ end, is }) => {
   end()
 })
 
-test('passes send function messages in logEvent object', ({ end, same, is }) => {
+test('passes send function message strings in logEvent object when asObject is not set', ({ end, same, is }) => {
   const logger = pino({
     browser: {
+      write: noop,
+      transmit: {
+        send (level, { messages }) {
+          is(messages[0], 'test')
+          is(messages[1], 'another test')
+        }
+      }
+    }
+  })
+
+  logger.fatal('test', 'another test')
+
+  end()
+})
+
+test('passes send function message objects in logEvent object when asObject is not set', ({ end, same, is }) => {
+  const logger = pino({
+    browser: {
+      write: noop,
+      transmit: {
+        send (level, { messages }) {
+          same(messages[0], { test: 'test' })
+          is(messages[1], 'another test')
+        }
+      }
+    }
+  })
+
+  logger.fatal({ test: 'test' }, 'another test')
+
+  end()
+})
+
+test('passes send function message strings in logEvent object when asObject is set', ({ end, same, is }) => {
+  const logger = pino({
+    browser: {
+      asObject: true,
+      write: noop,
+      transmit: {
+        send (level, { messages }) {
+          is(messages[0], 'test')
+          is(messages[1], 'another test')
+        }
+      }
+    }
+  })
+
+  logger.fatal('test', 'another test')
+
+  end()
+})
+
+test('passes send function message objects in logEvent object when asObject is set', ({ end, same, is }) => {
+  const logger = pino({
+    browser: {
+      asObject: true,
       write: noop,
       transmit: {
         send (level, { messages }) {
@@ -63,6 +119,7 @@ test('passes send function messages in logEvent object', ({ end, same, is }) =>
   })
 
   logger.fatal({ test: 'test' }, 'another test')
+
   end()
 })
 
diff --git a/test/browser.test.js b/test/browser.test.js
index 8dfdeb3..0218448 100644
--- a/test/browser.test.js
+++ b/test/browser.test.js
@@ -198,6 +198,38 @@ test('opts.browser.write func string joining', ({ end, ok, is }) => {
   end()
 })
 
+test('opts.browser.write func string joining when asObject is true', ({ end, ok, is }) => {
+  const instance = pino({
+    browser: {
+      asObject: true,
+      write: function (o) {
+        is(o.level, 30)
+        is(o.msg, 'test test2 test3')
+        ok(o.time)
+      }
+    }
+  })
+  instance.info('test', 'test2', 'test3')
+
+  end()
+})
+
+test('opts.browser.write func string joining when asObject is true', ({ end, ok, is }) => {
+  const instance = pino({
+    browser: {
+      asObject: true,
+      write: function (o) {
+        is(o.level, 30)
+        is(o.msg, 'test test2 test3')
+        ok(o.time)
+      }
+    }
+  })
+  instance.info('test', 'test2', 'test3')
+
+  end()
+})
+
 test('opts.browser.write func string object joining', ({ end, ok, is }) => {
   const instance = pino({
     browser: {
@@ -213,6 +245,22 @@ test('opts.browser.write func string object joining', ({ end, ok, is }) => {
   end()
 })
 
+test('opts.browser.write func string object joining when asObject is true', ({ end, ok, is }) => {
+  const instance = pino({
+    browser: {
+      asObject: true,
+      write: function (o) {
+        is(o.level, 30)
+        is(o.msg, 'test {"test":"test2"} {"test":"test3"}')
+        ok(o.time)
+      }
+    }
+  })
+  instance.info('test', { test: 'test2' }, { test: 'test3' })
+
+  end()
+})
+
 test('opts.browser.write func string interpolation', ({ end, ok, is }) => {
   const instance = pino({
     browser: {