Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Never serialize .cause Errors, rely on message + stack summary instead #110

Merged
merged 5 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions lib/err-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

/**
Expand Down Expand Up @@ -107,7 +108,6 @@ const messageWithCauses = (err) => _messageWithCauses(err, new Set())

module.exports = {
getErrorCause,
evaluateCause,
stackWithCauses,
messageWithCauses
}
18 changes: 4 additions & 14 deletions lib/err.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -28,11 +28,6 @@ const pinoErrProto = Object.create({}, {
writable: true,
value: undefined
},
cause: {
enumerable: true,
writable: true,
value: undefined
},
raw: {
enumerable: false,
get: function () {
Expand Down Expand Up @@ -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 {
Expand Down
70 changes: 15 additions & 55 deletions test/err.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
Expand All @@ -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')
Expand Down Expand Up @@ -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:/)
})