Never serialize .cause Errors, rely on message + stack summary inst…

…ead (#110)

* Revert "Nested VError causes (#108)"

This reverts commit 5b88f7e.

* Revert "Serialize errors with cause (#105)"

This reverts commit ae83956.

* Never serialize `.cause` when an `Error`

Instead rely on its content being appended to the message and the stack.

This is an alternative fix to #94, replacing #105 and #108 + makes #109 not needed

* Add tests

* Add explainer comment

lib/err-helpers.js

@@ -12,22 +12,23 @@
 const getErrorCause = (err) => {
   if (!err) return
 
-  const cause = evaluateCause(err.cause)
+  /** @type {unknown} */
+  // @ts-ignore
+  const cause = err.cause
 
-  return cause instanceof Error
-    ? cause
-    : undefined
-}
+  // VError / NError style causes
+  if (typeof cause === 'function') {
+    // @ts-ignore
+    const causeResult = err.cause()
 
-/**
- * @param {unknown|(()=>err)} cause
- * @returns {Error|undefined}
- */
-const evaluateCause = (cause) => {
-  // VError / NError style causes are functions
-  return typeof cause === 'function'
-    ? cause()
-    : cause
+    return causeResult instanceof Error
+      ? causeResult
+      : undefined
+  } else {
+    return cause instanceof Error
+      ? cause
+      : undefined
+  }
 }
 
 /**
@@ -107,7 +108,6 @@ const messageWithCauses = (err) => _messageWithCauses(err, new Set())
 
 module.exports = {
   getErrorCause,
-  evaluateCause,
   stackWithCauses,
   messageWithCauses
 }

lib/err.js

@@ -2,7 +2,7 @@
 
 module.exports = errSerializer
 
-const { messageWithCauses, stackWithCauses, evaluateCause } = require('./err-helpers')
+const { messageWithCauses, stackWithCauses } = require('./err-helpers')
 
 const { toString } = Object.prototype
 const seen = Symbol('circular-ref-tag')
@@ -28,11 +28,6 @@ const pinoErrProto = Object.create({}, {
     writable: true,
     value: undefined
   },
-  cause: {
-    enumerable: true,
-    writable: true,
-    value: undefined
-  },
   raw: {
     enumerable: false,
     get: function () {
@@ -65,17 +60,12 @@ function errSerializer (err) {
     _err.aggregateErrors = err.errors.map(err => errSerializer(err))
   }
 
-  if (err.cause) {
-    const cause = evaluateCause(err.cause)
-    _err.cause = errSerializer(cause)
-  }
-
   for (const key in err) {
     if (_err[key] === undefined) {
       const val = err[key]
-      if (val instanceof Error && key !== 'cause') {
-        /* eslint-disable no-prototype-builtins */
-        if (!val.hasOwnProperty(seen)) {
+      if (val instanceof Error) {
+        // We append cause messages and stacks to _err, therefore skipping causes here
+        if (key !== 'cause' && !Object.prototype.hasOwnProperty.call(val, seen)) {
           _err[key] = errSerializer(val)
         }
       } else {

test/err.test.js

@@ -47,7 +47,7 @@ test('serializes nested errors', function (t) {
 })
 
 test('serializes error causes', function (t) {
-  t.plan(6)
+  t.plan(7)
   const err = Error('foo')
   err.cause = Error('bar')
   err.cause.cause = Error('abc')
@@ -58,15 +58,17 @@ test('serializes error causes', function (t) {
   t.match(serialized.stack, /Error: foo/)
   t.match(serialized.stack, /Error: bar/)
   t.match(serialized.stack, /Error: abc/)
+  t.notOk(serialized.cause)
 })
 
 test('serializes error causes with VError support', function (t) {
   t.plan(6)
   // Fake VError-style setup
   const err = Error('foo: bar')
-  err.cause = () => {
+  err.foo = 'abc'
+  err.cause = function () {
     const err = Error('bar')
-    err.cause = Error('abc')
+    err.cause = Error(this.foo)
     return err
   }
   const serialized = serializer(err)
@@ -78,6 +80,16 @@ test('serializes error causes with VError support', function (t) {
   t.match(serialized.stack, /Error: abc/)
 })
 
+test('keeps non-error cause', function (t) {
+  t.plan(3)
+  const err = Error('foo')
+  err.cause = 'abc'
+  const serialized = serializer(err)
+  t.equal(serialized.type, 'Error')
+  t.equal(serialized.message, 'foo')
+  t.equal(serialized.cause, 'abc')
+})
+
 test('prevents infinite recursion', function (t) {
   t.plan(4)
   const err = Error('foo')
@@ -194,55 +206,3 @@ test('serializes aggregate errors', { skip: !global.AggregateError }, function (
   t.match(serialized.aggregateErrors[1].stack, /^Error: bar/)
   t.match(serialized.stack, /err\.test\.js:/)
 })
-
-test('serializes causes', function (t) {
-  t.plan(11)
-
-  const bar = new Error('bar')
-  bar.cause = new Error('foo')
-  bar.cause.cause = new Error('baz')
-
-  const serialized = serializer(bar)
-
-  t.equal(serialized.type, 'Error')
-  t.equal(serialized.message, 'bar: foo: baz') // message serialization already walks cause-chain
-  t.match(serialized.stack, /err\.test\.js:/)
-
-  t.ok(serialized.cause)
-  t.equal(serialized.cause.type, 'Error')
-  t.equal(serialized.cause.message, 'foo: baz')
-  t.match(serialized.cause.stack, /err\.test\.js:/)
-
-  t.ok(serialized.cause.cause)
-  t.equal(serialized.cause.cause.type, 'Error')
-  t.equal(serialized.cause.cause.message, 'baz')
-  t.match(serialized.cause.cause.stack, /err\.test\.js:/)
-})
-
-test('serializes causes with VError support', function (t) {
-  t.plan(11)
-
-  // Fake VError-style setup
-  const err = Error('foo: bar')
-  err.cause = () => {
-    const err = Error('bar')
-    err.cause = Error('abc')
-    return err
-  }
-
-  const serialized = serializer(err)
-
-  t.equal(serialized.type, 'Error')
-  t.equal(serialized.message, 'foo: bar: abc') // message serialization already walks cause-chain
-  t.match(serialized.stack, /err\.test\.js:/)
-
-  t.ok(serialized.cause)
-  t.equal(serialized.cause.type, 'Error')
-  t.equal(serialized.cause.message, 'bar: abc')
-  t.match(serialized.cause.stack, /err\.test\.js:/)
-
-  t.ok(serialized.cause.cause)
-  t.equal(serialized.cause.cause.type, 'Error')
-  t.equal(serialized.cause.cause.message, 'abc')
-  t.match(serialized.cause.cause.stack, /err\.test\.js:/)
-})