From db905bb44554c5c7d2862b60a8a685ffda0ee29a Mon Sep 17 00:00:00 2001 From: Yuval Peled Date: Wed, 29 Mar 2023 15:56:08 +0300 Subject: [PATCH 1/6] feat(serializers): Implemented "error with cause" serializer - Created new serializer called "errWithCause". This serializer acts very similarly to the default "err" serializer. The differences: When it encounters an "error.cause" field, it serializes it recursively to create a nested object of errors. This is as opposed to the default error serializer, in which the resulting serialized object has no "cause" field, and the causes' messages & stack traces are appended to the original error.message and error.stack - Since some logic is reused between the existing err serializer and the new one, I moved some logic to the err-helpers file - Created a test-file for the new serializer. It is very similar to the existing err serializer test file since they should behave nearly identically. The differences are in the tests that test for the "cause" related behavior. - Upgrade tsconfig.json "lib" to support error.cause fields. #126 --- index.d.ts | 8 +- index.js | 2 + lib/err-helpers.js | 45 +++++++- lib/err-with-cause.js | 47 +++++++++ lib/err.js | 40 +------ test/err-with-cause.test.js | 201 ++++++++++++++++++++++++++++++++++++ test/types/index.test-d.ts | 4 + tsconfig.json | 2 +- 8 files changed, 308 insertions(+), 41 deletions(-) create mode 100644 lib/err-with-cause.js create mode 100644 test/err-with-cause.test.js diff --git a/index.d.ts b/index.d.ts index d28e017..5133c73 100644 --- a/index.d.ts +++ b/index.d.ts @@ -32,10 +32,16 @@ export interface SerializedError { } /** - * Serializes an Error object. + * Serializes an Error object. Does not serialize "err.cause" fields (will append the err.cause.message to err.message + * and err.cause.stack to err.stack) */ export function err(err: Error): SerializedError; +/** + * Serializes an Error object, including full serialization for any err.cause fields recursively. + */ +export function errWithCause(err: Error): SerializedError; + export interface SerializedRequest { /** * Defaults to `undefined`, unless there is an `id` property already attached to the `request` object or diff --git a/index.js b/index.js index 88f6027..ef2b660 100644 --- a/index.js +++ b/index.js @@ -1,11 +1,13 @@ 'use strict' const errSerializer = require('./lib/err') +const errWithCauseSerializer = require('./lib/err-with-cause') const reqSerializers = require('./lib/req') const resSerializers = require('./lib/res') module.exports = { err: errSerializer, + errWithCause: errWithCauseSerializer, mapHttpRequest: reqSerializers.mapHttpRequest, mapHttpResponse: resSerializers.mapHttpResponse, req: reqSerializers.reqSerializer, diff --git a/lib/err-helpers.js b/lib/err-helpers.js index efdec2c..50c54f3 100644 --- a/lib/err-helpers.js +++ b/lib/err-helpers.js @@ -110,9 +110,52 @@ const _messageWithCauses = (err, seen, skip) => { */ const messageWithCauses = (err) => _messageWithCauses(err, new Set()) +const seen = Symbol('circular-ref-tag') +const rawSymbol = Symbol('pino-raw-err-ref') +const pinoErrProto = Object.create({}, { + type: { + enumerable: true, + writable: true, + value: undefined + }, + message: { + enumerable: true, + writable: true, + value: undefined + }, + stack: { + enumerable: true, + writable: true, + value: undefined + }, + aggregateErrors: { + enumerable: true, + writable: true, + value: undefined + }, + raw: { + enumerable: false, + get: function () { + return this[rawSymbol] + }, + set: function (val) { + this[rawSymbol] = val + } + } +}) +Object.defineProperty(pinoErrProto, rawSymbol, { + writable: true, + value: {} +}) + module.exports = { isErrorLike, getErrorCause, stackWithCauses, - messageWithCauses + messageWithCauses, + pinoErrProto, + pinoErrorSymbols: { + seen, + rawSymbol + } } diff --git a/lib/err-with-cause.js b/lib/err-with-cause.js new file mode 100644 index 0000000..fcaae68 --- /dev/null +++ b/lib/err-with-cause.js @@ -0,0 +1,47 @@ +'use strict' + +module.exports = errWithCauseSerializer + +const { isErrorLike, pinoErrProto, pinoErrorSymbols } = require('./err-helpers') +const { seen } = pinoErrorSymbols + +const { toString } = Object.prototype + +function errWithCauseSerializer (err) { + if (!isErrorLike(err)) { + return err + } + + err[seen] = undefined // tag to prevent re-looking at this + const _err = Object.create(pinoErrProto) + _err.type = toString.call(err.constructor) === '[object Function]' + ? err.constructor.name + : err.name + _err.message = err.message + _err.stack = err.stack + + if (Array.isArray(err.errors)) { + _err.aggregateErrors = err.errors.map(err => errWithCauseSerializer(err)) + } + + if (isErrorLike(err.cause) && !Object.prototype.hasOwnProperty.call(err.cause, seen)) { + _err.cause = errWithCauseSerializer(err.cause) + } + + for (const key in err) { + if (_err[key] === undefined) { + const val = err[key] + if (isErrorLike(val)) { + if (!Object.prototype.hasOwnProperty.call(val, seen)) { + _err[key] = errWithCauseSerializer(val) + } + } else { + _err[key] = val + } + } + } + + delete err[seen] // clean up tag in case err is serialized again later + _err.raw = err + return _err +} diff --git a/lib/err.js b/lib/err.js index 96c5775..7f96b43 100644 --- a/lib/err.js +++ b/lib/err.js @@ -2,46 +2,10 @@ module.exports = errSerializer -const { messageWithCauses, stackWithCauses, isErrorLike } = require('./err-helpers') +const { messageWithCauses, stackWithCauses, isErrorLike, pinoErrProto, pinoErrorSymbols } = require('./err-helpers') +const { seen } = pinoErrorSymbols const { toString } = Object.prototype -const seen = Symbol('circular-ref-tag') -const rawSymbol = Symbol('pino-raw-err-ref') -const pinoErrProto = Object.create({}, { - type: { - enumerable: true, - writable: true, - value: undefined - }, - message: { - enumerable: true, - writable: true, - value: undefined - }, - stack: { - enumerable: true, - writable: true, - value: undefined - }, - aggregateErrors: { - enumerable: true, - writable: true, - value: undefined - }, - raw: { - enumerable: false, - get: function () { - return this[rawSymbol] - }, - set: function (val) { - this[rawSymbol] = val - } - } -}) -Object.defineProperty(pinoErrProto, rawSymbol, { - writable: true, - value: {} -}) function errSerializer (err) { if (!isErrorLike(err)) { diff --git a/test/err-with-cause.test.js b/test/err-with-cause.test.js new file mode 100644 index 0000000..34381f7 --- /dev/null +++ b/test/err-with-cause.test.js @@ -0,0 +1,201 @@ +'use strict' + +const test = require('tap').test +const serializer = require('../lib/err-with-cause') +const wrapErrorSerializer = require('../').wrapErrorSerializer + +test('serializes Error objects', function (t) { + t.plan(3) + const serialized = serializer(Error('foo')) + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.match(serialized.stack, /err-with-cause\.test\.js:/) +}) + +test('serializes Error objects with extra properties', function (t) { + t.plan(5) + const err = Error('foo') + err.statusCode = 500 + const serialized = serializer(err) + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.ok(serialized.statusCode) + t.equal(serialized.statusCode, 500) + t.match(serialized.stack, /err-with-cause\.test\.js:/) +}) + +test('serializes Error objects with subclass "type"', function (t) { + t.plan(1) + + class MyError extends Error {} + + const err = new MyError('foo') + const serialized = serializer(err) + t.equal(serialized.type, 'MyError') +}) + +test('serializes nested errors', function (t) { + t.plan(7) + const err = Error('foo') + err.inner = Error('bar') + const serialized = serializer(err) + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.match(serialized.stack, /err-with-cause\.test\.js:/) + t.equal(serialized.inner.type, 'Error') + t.equal(serialized.inner.message, 'bar') + t.match(serialized.inner.stack, /Error: bar/) + t.match(serialized.inner.stack, /err-with-cause\.test\.js:/) +}) + +test('serializes error causes', function (t) { + const innerErr = Error('inner') + const middleErr = Error('middle', { cause: innerErr }) + const outerErr = Error('outer', { cause: middleErr }) + + const serialized = serializer(outerErr) + + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'outer') + t.match(serialized.stack, /err-with-cause\.test\.js:/) + + t.equal(serialized.cause.type, 'Error') + t.equal(serialized.cause.message, 'middle') + t.match(serialized.cause.stack, /err-with-cause\.test\.js:/) + + t.equal(serialized.cause.cause.type, 'Error') + t.equal(serialized.cause.cause.message, 'inner') + t.match(serialized.cause.cause.stack, /err-with-cause\.test\.js:/) + + t.end() +}) + +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') + err.inner = err + const serialized = serializer(err) + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.match(serialized.stack, /err-with-cause\.test\.js:/) + t.notOk(serialized.inner) +}) + +test('cleans up infinite recursion tracking', function (t) { + t.plan(8) + const err = Error('foo') + const bar = Error('bar') + err.inner = bar + bar.inner = err + + serializer(err) + const serialized = serializer(err) + + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.match(serialized.stack, /err-with-cause\.test\.js:/) + t.ok(serialized.inner) + t.equal(serialized.inner.type, 'Error') + t.equal(serialized.inner.message, 'bar') + t.match(serialized.inner.stack, /Error: bar/) + t.notOk(serialized.inner.inner) +}) + +test('err.raw is available', function (t) { + t.plan(1) + const err = Error('foo') + const serialized = serializer(err) + t.equal(serialized.raw, err) +}) + +test('redefined err.constructor doesnt crash serializer', function (t) { + t.plan(10) + + function check (a, name) { + t.equal(a.type, name) + t.equal(a.message, 'foo') + } + + const err1 = TypeError('foo') + err1.constructor = '10' + + const err2 = TypeError('foo') + err2.constructor = undefined + + const err3 = Error('foo') + err3.constructor = null + + const err4 = Error('foo') + err4.constructor = 10 + + class MyError extends Error {} + + const err5 = new MyError('foo') + err5.constructor = undefined + + check(serializer(err1), 'TypeError') + check(serializer(err2), 'TypeError') + check(serializer(err3), 'Error') + check(serializer(err4), 'Error') + // We do not expect 'MyError' because err5.constructor has been blown away. + // `err5.name` is 'Error' from the base class prototype. + check(serializer(err5), 'Error') +}) + +test('pass through anything that does not look like an Error', function (t) { + t.plan(3) + + function check (a) { + t.equal(serializer(a), a) + } + + check('foo') + check({ hello: 'world' }) + check([1, 2]) +}) + +test('can wrap err serializers', function (t) { + t.plan(5) + const err = Error('foo') + err.foo = 'foo' + const serializer = wrapErrorSerializer(function (err) { + delete err.foo + err.bar = 'bar' + return err + }) + const serialized = serializer(err) + t.equal(serialized.type, 'Error') + t.equal(serialized.message, 'foo') + t.match(serialized.stack, /err-with-cause\.test\.js:/) + t.notOk(serialized.foo) + t.equal(serialized.bar, 'bar') +}) + +test('serializes aggregate errors', { skip: !global.AggregateError }, function (t) { + t.plan(14) + const foo = new Error('foo') + const bar = new Error('bar') + for (const aggregate of [ + new AggregateError([foo, bar], 'aggregated message'), // eslint-disable-line no-undef + { errors: [foo, bar], message: 'aggregated message', stack: 'err-with-cause.test.js:' } + ]) { + const serialized = serializer(aggregate) + t.equal(serialized.message, 'aggregated message') + t.equal(serialized.aggregateErrors.length, 2) + t.equal(serialized.aggregateErrors[0].message, 'foo') + t.equal(serialized.aggregateErrors[1].message, 'bar') + t.match(serialized.aggregateErrors[0].stack, /^Error: foo/) + t.match(serialized.aggregateErrors[1].stack, /^Error: bar/) + t.match(serialized.stack, /err-with-cause\.test\.js:/) + } +}) diff --git a/test/types/index.test-d.ts b/test/types/index.test-d.ts index 0419828..9d0646e 100644 --- a/test/types/index.test-d.ts +++ b/test/types/index.test-d.ts @@ -1,6 +1,7 @@ import {IncomingMessage, ServerResponse} from "http"; import { err, + errWithCause, req, res, SerializedError, @@ -45,6 +46,9 @@ const fakeError = new Error('A fake error for testing'); const serializedError: SerializedError = err(fakeError); const mySerializer = wrapErrorSerializer(customErrorSerializer); +const fakeErrorWithCause = new Error('A fake error for testing with cause', { cause: new Error('An inner fake error') }); +const serializedErrorWithCause: SerializedError = errWithCause(fakeError); + const request: IncomingMessage = {} as IncomingMessage const serializedRequest: SerializedRequest = req(request); const myReqSerializer = wrapRequestSerializer(customRequestSerializer); diff --git a/tsconfig.json b/tsconfig.json index 8e0b9fa..d3be182 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,7 +1,7 @@ { "compilerOptions": { "target": "es6", - "lib": [ "es2015" ], + "lib": [ "es2022" ], "module": "commonjs", "noEmit": true, "strict": true From 279678eb29f9941914e22fc13ece995028968bc3 Mon Sep 17 00:00:00 2001 From: Yuval Peled Date: Wed, 29 Mar 2023 17:39:33 +0300 Subject: [PATCH 2/6] fix test to work with node v14 that does not support Error constructor with 2nd argument --- test/err-with-cause.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/err-with-cause.test.js b/test/err-with-cause.test.js index 34381f7..bb87a56 100644 --- a/test/err-with-cause.test.js +++ b/test/err-with-cause.test.js @@ -50,8 +50,10 @@ test('serializes nested errors', function (t) { test('serializes error causes', function (t) { const innerErr = Error('inner') - const middleErr = Error('middle', { cause: innerErr }) - const outerErr = Error('outer', { cause: middleErr }) + const middleErr = Error('middle') + middleErr.cause = innerErr + const outerErr = Error('outer') + outerErr.cause = middleErr const serialized = serializer(outerErr) From acaf19482b66a2b3640d33505c10403a89f94208 Mon Sep 17 00:00:00 2001 From: Yuval Peled Date: Wed, 29 Mar 2023 17:53:11 +0300 Subject: [PATCH 3/6] Code review fix: Moved error prototype related code to its own file --- lib/err-helpers.js | 45 +------------------------------------------ lib/err-proto.js | 45 +++++++++++++++++++++++++++++++++++++++++++ lib/err-with-cause.js | 3 ++- lib/err.js | 3 ++- 4 files changed, 50 insertions(+), 46 deletions(-) create mode 100644 lib/err-proto.js diff --git a/lib/err-helpers.js b/lib/err-helpers.js index 50c54f3..efdec2c 100644 --- a/lib/err-helpers.js +++ b/lib/err-helpers.js @@ -110,52 +110,9 @@ const _messageWithCauses = (err, seen, skip) => { */ const messageWithCauses = (err) => _messageWithCauses(err, new Set()) -const seen = Symbol('circular-ref-tag') -const rawSymbol = Symbol('pino-raw-err-ref') -const pinoErrProto = Object.create({}, { - type: { - enumerable: true, - writable: true, - value: undefined - }, - message: { - enumerable: true, - writable: true, - value: undefined - }, - stack: { - enumerable: true, - writable: true, - value: undefined - }, - aggregateErrors: { - enumerable: true, - writable: true, - value: undefined - }, - raw: { - enumerable: false, - get: function () { - return this[rawSymbol] - }, - set: function (val) { - this[rawSymbol] = val - } - } -}) -Object.defineProperty(pinoErrProto, rawSymbol, { - writable: true, - value: {} -}) - module.exports = { isErrorLike, getErrorCause, stackWithCauses, - messageWithCauses, - pinoErrProto, - pinoErrorSymbols: { - seen, - rawSymbol - } + messageWithCauses } diff --git a/lib/err-proto.js b/lib/err-proto.js new file mode 100644 index 0000000..5e5f74a --- /dev/null +++ b/lib/err-proto.js @@ -0,0 +1,45 @@ +const seen = Symbol('circular-ref-tag') +const rawSymbol = Symbol('pino-raw-err-ref') +const pinoErrProto = Object.create({}, { + type: { + enumerable: true, + writable: true, + value: undefined + }, + message: { + enumerable: true, + writable: true, + value: undefined + }, + stack: { + enumerable: true, + writable: true, + value: undefined + }, + aggregateErrors: { + enumerable: true, + writable: true, + value: undefined + }, + raw: { + enumerable: false, + get: function () { + return this[rawSymbol] + }, + set: function (val) { + this[rawSymbol] = val + } + } +}) +Object.defineProperty(pinoErrProto, rawSymbol, { + writable: true, + value: {} +}) + +module.exports = { + pinoErrProto, + pinoErrorSymbols: { + seen, + rawSymbol + } +} diff --git a/lib/err-with-cause.js b/lib/err-with-cause.js index fcaae68..29939e0 100644 --- a/lib/err-with-cause.js +++ b/lib/err-with-cause.js @@ -2,7 +2,8 @@ module.exports = errWithCauseSerializer -const { isErrorLike, pinoErrProto, pinoErrorSymbols } = require('./err-helpers') +const { isErrorLike } = require('./err-helpers') +const { pinoErrProto, pinoErrorSymbols } = require('./err-proto') const { seen } = pinoErrorSymbols const { toString } = Object.prototype diff --git a/lib/err.js b/lib/err.js index 7f96b43..338b230 100644 --- a/lib/err.js +++ b/lib/err.js @@ -2,7 +2,8 @@ module.exports = errSerializer -const { messageWithCauses, stackWithCauses, isErrorLike, pinoErrProto, pinoErrorSymbols } = require('./err-helpers') +const { messageWithCauses, stackWithCauses, isErrorLike } = require('./err-helpers') +const { pinoErrProto, pinoErrorSymbols } = require('./err-proto') const { seen } = pinoErrorSymbols const { toString } = Object.prototype From 1256c279dc4dee151aa44a99e73b717916e2f9ef Mon Sep 17 00:00:00 2001 From: Yuval Peled <31162840+Yuval-Peled@users.noreply.github.com> Date: Wed, 29 Mar 2023 17:58:54 +0300 Subject: [PATCH 4/6] Add "use strict" and fix whitespace according to style According to code review by @jsumners Co-authored-by: James Sumners --- lib/err-proto.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/err-proto.js b/lib/err-proto.js index 5e5f74a..a01447d 100644 --- a/lib/err-proto.js +++ b/lib/err-proto.js @@ -1,5 +1,8 @@ +'use strict' + const seen = Symbol('circular-ref-tag') const rawSymbol = Symbol('pino-raw-err-ref') + const pinoErrProto = Object.create({}, { type: { enumerable: true, From c24daa97455e4c407484a93b35bc14511907abdc Mon Sep 17 00:00:00 2001 From: Yuval Peled Date: Wed, 29 Mar 2023 19:15:44 +0300 Subject: [PATCH 5/6] Add documentation for errWithCause --- Readme.md | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/Readme.md b/Readme.md index 67f4a52..d0cc961 100644 --- a/Readme.md +++ b/Readme.md @@ -16,11 +16,81 @@ Serializes an `Error` like object. Returns an object: raw: Error // Non-enumerable, i.e. will not be in the output, original // Error object. This is available for subsequent serializers // to use. + [...any additional Enumerable property the original Error had] } ``` Any other extra properties, e.g. `statusCode`, that have been attached to the object will also be present on the serialized object. +If the error object has a [`cause`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause) property, the `cause`'s `message` and `stack` will be appended to the top-level `message` and `stack`. All other parameters that belong to the `error.cause` object will be omitted. +Example: +```javascript +const serializer = require('pino-std-serializers').err; + +const innerError = new Error("inner error"); +innerError.isInner = true; +const outerError = new Error("outer error", { cause: innerError }); +outerError.isInner = false; + +const serialized = serializer(outerError); +/* Result: +{ + "type": "Error", + "message": "outer error: inner error", + "isInner": false, + "stack": "Error: outer error + at <...omitted..> + caused by: Error: inner error + at <...omitted..> +} + */ +``` + +### `exports.errWithCause(error)` +Serializes an `Error` like object, including any `error.cause`. Returns an object: + +```js +{ + type: 'string', // The name of the object's constructor. + message: 'string', // The supplied error message. + stack: 'string', // The stack when the error was generated. + cause?: Error, // If the original error had an error.cause, it will be serialized here + raw: Error // Non-enumerable, i.e. will not be in the output, original + // Error object. This is available for subsequent serializers + // to use. + [...any additional Enumerable property the original Error had] +} +``` + +Any other extra properties, e.g. `statusCode`, that have been attached to the object will also be present on the serialized object. + +Example: +```javascript +const serializer = require('pino-std-serializers').errWithCause; + +const innerError = new Error("inner error"); +innerError.isInner = true; +const outerError = new Error("outer error", { cause: innerError }); +outerError.isInner = false; + +const serialized = serializer(outerError); +/* Result: +{ + "type": "Error", + "message": "outer error", + "isInner": false, + "stack": "Error: outer error + at <...omitted..>", + "cause": { + "type": "Error", + "message": "inner error", + "isInner": true, + "stack": "Error: inner error + at <...omitted..>" + }, +} + */ +``` ### `exports.mapHttpResponse(response)` Used internally by Pino for general response logging. Returns an object: @@ -49,7 +119,7 @@ The default `request` serializer. Returns an object: ```js { - id: 'string', // Defaults to `undefined`, unless there is an `id` property + id: 'string', // Defaults to `undefined`, unless there is an `id` property // already attached to the `request` object or to the `request.info` // object. Attach a synchronous function // to the `request.id` that returns an identifier to have @@ -64,7 +134,7 @@ The default `request` serializer. Returns an object: remotePort: Number, raw: Object // Non-enumerable, i.e. will not be in the output, original // request object. This is available for subsequent serializers - // to use. In cases where the `request` input already has + // to use. In cases where the `request` input already has // a `raw` property this will replace the original `request.raw` // property } From 2751985ee9a7bdc7254a5b88e31d4b6cf177a623 Mon Sep 17 00:00:00 2001 From: Yuval Peled <31162840+Yuval-Peled@users.noreply.github.com> Date: Mon, 3 Apr 2023 11:25:01 +0300 Subject: [PATCH 6/6] Update Readme.md according to code review suggestion Co-authored-by: James Sumners --- Readme.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Readme.md b/Readme.md index d0cc961..fa22b1f 100644 --- a/Readme.md +++ b/Readme.md @@ -22,9 +22,12 @@ Serializes an `Error` like object. Returns an object: Any other extra properties, e.g. `statusCode`, that have been attached to the object will also be present on the serialized object. + If the error object has a [`cause`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause) property, the `cause`'s `message` and `stack` will be appended to the top-level `message` and `stack`. All other parameters that belong to the `error.cause` object will be omitted. + Example: -```javascript + +```js const serializer = require('pino-std-serializers').err; const innerError = new Error("inner error"); @@ -44,7 +47,6 @@ const serialized = serializer(outerError); at <...omitted..> } */ -``` ### `exports.errWithCause(error)` Serializes an `Error` like object, including any `error.cause`. Returns an object: