[fix] Better handling of new Error(string) throughout the pipeline(s). Fixes #1338, #1486 (#1562)

* [tiny dist] Whitespace. package-lock.json

* [test] Add E2E integration tests with logform `errors` format.

* [test] 5:00pm. Press return.

* [fix test] More E2E errors coverage.

* [test] Test custom error properties.

* [tiny doc] Make note of duplicate coverage in `logger`. Update minor formatting.

* [test] All E2E tests work except for one...

* [fix test doc] All 14 variations of handling `new Error()` now work as most folks expect.

* [tiny] Fix up file header.

* [dist] Bump to `logform@2.1.0`

* [fix tiny] Whitespace.

* s/req_id/requestId/

* [fix test] Address PR comments. Add test coverage for defaultMeta over .child(additionalMeta)

Author: indexzero

README.md

@@ -203,7 +203,7 @@ const logger = winston.createLogger({
   ]
 });
 
-const childLogger = logger.child({ req_id: '451' });
+const childLogger = logger.child({ requestId: '451' });
 ```
 
 ### Streams, `objectMode`, and `info` objects
@@ -235,19 +235,13 @@ treated as immutable by all code.
 - `Symbol.for('message'):` complete string message set by "finalizing 
 formats": `json`, `logstash`, `printf`, `prettyPrint`, and `simple`. 
 
-> **NOTE:** the `message` and `level` properties are considered reserved.
-> Please be aware of this when logging additional metadata objects. For 
-> example the below will suppress the `message` property of the metadata
-> provided:
+> **NOTE:** any `{ message }` property in a `meta` object provided will
+> automatically be concatenated to any `msg` already provided: For 
+> example the below will concatenate 'world' onto 'hello':
 >
 > ``` js
-> logger.log('hello', { message: 'will be hidden' });
-> ```
-> 
-> To work around this use the `splat()` format below. e.g.:
-> 
-> ``` js
-> logger.log('hello %j', { message: 'will be shown' });
+> logger.log('error', 'hello', { message: 'world' });
+> logger.info('hello', { message: 'world' });
 > ```
 
 ## Formats

examples/errors.js

@@ -0,0 +1,20 @@
+const { createLogger, format, transports } = require('../');
+const { combine, errors, json } = format;
+
+const logger = createLogger({
+  format: combine(
+    errors({ stack: true }),
+    json()
+  ),
+  transports: [
+    new transports.Console(),
+  ]
+});
+
+logger.warn(new Error('Error passed as info'));
+logger.log('error', new Error('Error passed as message'));
+
+logger.warn('Maybe important error: ', new Error('Error passed as meta'));
+logger.log('error', 'Important error: ', new Error('Error passed as meta'));
+
+logger.error(new Error('Error as info'));

lib/winston/logger.js

@@ -33,14 +33,12 @@ const formatRegExp = /%[scdjifoO%]/g;
  */
 class Logger extends Transform {
   /**
-     * Constructor function for the Logger object responsible for persisting log
-     * messages and metadata to one or more transports.
-     * @param {!Object} options - foo
-     */
+   * Constructor function for the Logger object responsible for persisting log
+   * messages and metadata to one or more transports.
+   * @param {!Object} options - foo
+   */
   constructor(options) {
-    super({
-      objectMode: true
-    });
+    super({ objectMode: true });
     this.configure(options);
   }
 
@@ -55,7 +53,12 @@ class Logger extends Transform {
             info
           );
 
-          // Object.assign doesn't copy inherited Error properties so we have to do that explicitly
+          // Object.assign doesn't copy inherited Error
+          // properties so we have to do that explicitly
+          //
+          // Remark (indexzero): we should remove this
+          // since the errors format will handle this case.
+          //
           if (info instanceof Error) {
             infoClone.stack = info.stack;
             infoClone.message = info.message;
@@ -228,29 +231,28 @@ class Logger extends Transform {
       const tokens = msg && msg.match && msg.match(formatRegExp);
 
       if (!tokens) {
-        this.write(Object.assign({}, meta, {
-          [LEVEL]: level,
-          [SPLAT]: splat,
-          level,
-          message: msg
-        }, this.defaultMeta));
-      } else {
-        this.write(Object.assign({}, {
+        const info = Object.assign({}, this.defaultMeta, meta, {
           [LEVEL]: level,
           [SPLAT]: splat,
           level,
           message: msg
-        }, this.defaultMeta));
+        });
+
+        if (meta.message) info.message += `${meta.message}`;
+        if (meta.stack) info.stack = meta.stack;
+
+        this.write(info);
+        return this;
       }
-    } else {
-      this.write(Object.assign({}, {
-        [LEVEL]: level,
-        [SPLAT]: splat,
-        level,
-        message: msg
-      }, this.defaultMeta));
     }
 
+    this.write(Object.assign({}, this.defaultMeta, {
+      [LEVEL]: level,
+      [SPLAT]: splat,
+      level,
+      message: msg
+    }));
+
     return this;
   }