Preserve nested objects in failure payloads

[nicoburns]

Context

I am trying to save additional context and debugging information to the completion task in the case of a failed task, using code such as the following:

const error = new Error('Something went wrong');
error.additionalContext = foo;
throw err;

Bug

This works fine if foo is a number, string, or array. However if foo is a non-array object then pg-boss will convert it to the empty object. So if I do:

const foo = { bar: 'baz' };
const error = new Error('Something went wrong');
error.additionalContext = foo;
throw err;

I will end up with:

{
     "message": "Something went wrong",
     "stack": "...",
     "foo": {}
}

in the response property of my completion task.

Cause of the bug

This is caused by the following line in the mapCompletionDataArg function in manager.js:

if (data instanceof Error) { data = JSON.parse(JSON.stringify(data, Object.getOwnPropertyNames(data))) }

Here Object.getOwnPropertyNames(data) is an array of strings that functions as a replacer function, which means that only properties whose name is in the array will get serialised. This is a clever trick to ensure that the non-enumerable error and stack properties get serialized. Unfortunately the replacer function/array also gets applied to properties in any nested objects.

In our example above Object.getOwnPropertyNames(data) will return ['message', 'stack', 'foo']. As the string bar is not included in this array, when the { bar: 'baz' } object is filtered by the replacer array the bar property is filtered out (if the nested object had more properties, then any other properties that didn't happen to share a name with a property on the top-level object would also be filtered out).

Solution

I have replaced the problematic line above with the following code:

if (data instanceof Error) {
    const newData = {}
    Object.getOwnPropertyNames(data).forEach(key => { newData[key] = data[key] })
    data = newData
}

It uses a "dumb loop" to perform the object clone, avoiding the need for JSON.stringify entirely. I did it this way because I thought it would be faster, but it does mean that we have a shallow clone of the object rather than deep clone. I believe this shouldn't make any difference here, but if you did want a deep clone then you could run newData through a standard JSON.parse(JSON.serialize(...)) (no replacer needed) to get a deep clone which still retains the nested property data and the message and stack properties.

Notes

I've also added a test for this case, and a section to the README explaining how to setup and run the test suite.

[coveralls]

Coverage remained the same at 100.0% when pulling 7dbaf88e97793a06278798845ad52faa4c2c7dfd on nicoburns:preserve-nested-objects-in-failure-payloads into b091ca1cceca09ae1670c291c7e125c4b72f5f20 on timgit:master.

[timgit]

Thanks for the PR and detailed description. :+1: