From cb56e9f3344b33eab7a0145b81e0604f29daa5bf Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Mon, 13 Jun 2022 15:39:22 +0200 Subject: [PATCH 1/5] Revert "Nested VError causes (#108)" This reverts commit 5b88f7ee2597a035eb935b3d172bd3791f2c08db. --- lib/err-helpers.js | 30 +++++++++++++++--------------- lib/err.js | 5 ++--- test/err.test.js | 28 ---------------------------- 3 files changed, 17 insertions(+), 46 deletions(-) diff --git a/lib/err-helpers.js b/lib/err-helpers.js index d00c201..17b465f 100644 --- a/lib/err-helpers.js +++ b/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 } diff --git a/lib/err.js b/lib/err.js index fd63e23..d92fce4 100644 --- a/lib/err.js +++ b/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') @@ -66,8 +66,7 @@ function errSerializer (err) { } if (err.cause) { - const cause = evaluateCause(err.cause) - _err.cause = errSerializer(cause) + _err.cause = errSerializer(err.cause) } for (const key in err) { diff --git a/test/err.test.js b/test/err.test.js index 0ca4a58..d073c06 100644 --- a/test/err.test.js +++ b/test/err.test.js @@ -218,31 +218,3 @@ test('serializes causes', function (t) { 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:/) -}) From 23f62f2e0bc216f794bbbd1baef2941fd9918639 Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Mon, 13 Jun 2022 15:39:32 +0200 Subject: [PATCH 2/5] Revert "Serialize errors with cause (#105)" This reverts commit ae8395628a7520e8434106a08cfebb7cc44f5b29. --- lib/err.js | 9 --------- test/err.test.js | 24 ------------------------ 2 files changed, 33 deletions(-) diff --git a/lib/err.js b/lib/err.js index d92fce4..ccd81c9 100644 --- a/lib/err.js +++ b/lib/err.js @@ -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,10 +60,6 @@ function errSerializer (err) { _err.aggregateErrors = err.errors.map(err => errSerializer(err)) } - if (err.cause) { - _err.cause = errSerializer(err.cause) - } - for (const key in err) { if (_err[key] === undefined) { const val = err[key] diff --git a/test/err.test.js b/test/err.test.js index d073c06..746ed7a 100644 --- a/test/err.test.js +++ b/test/err.test.js @@ -194,27 +194,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:/) -}) From 8fd180977db2037b2820e8cd5b314bb6be4882c9 Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Mon, 13 Jun 2022 15:42:22 +0200 Subject: [PATCH 3/5] 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 --- lib/err.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/err.js b/lib/err.js index ccd81c9..764e2e4 100644 --- a/lib/err.js +++ b/lib/err.js @@ -63,9 +63,9 @@ function errSerializer (err) { for (const key in err) { if (_err[key] === undefined) { const val = err[key] - if (val instanceof Error && key !== 'cause') { + if (val instanceof Error) { /* eslint-disable no-prototype-builtins */ - if (!val.hasOwnProperty(seen)) { + if (key !== 'cause' && !val.hasOwnProperty(seen)) { _err[key] = errSerializer(val) } } else { From d848f45c82c7c858a13bf064ba313d2b799d5478 Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Mon, 13 Jun 2022 16:35:21 +0200 Subject: [PATCH 4/5] Add tests --- test/err.test.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/test/err.test.js b/test/err.test.js index 746ed7a..00a54d6 100644 --- a/test/err.test.js +++ b/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') From e785720c0b4a9a675cbf69e5865e800f9c62f32e Mon Sep 17 00:00:00 2001 From: Pelle Wessman Date: Mon, 13 Jun 2022 22:28:52 +0200 Subject: [PATCH 5/5] Add explainer comment --- lib/err.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/err.js b/lib/err.js index 764e2e4..c957394 100644 --- a/lib/err.js +++ b/lib/err.js @@ -64,8 +64,8 @@ function errSerializer (err) { if (_err[key] === undefined) { const val = err[key] if (val instanceof Error) { - /* eslint-disable no-prototype-builtins */ - if (key !== 'cause' && !val.hasOwnProperty(seen)) { + // 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 {