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.

Sign up for GitHub

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

Add support for AggregateError #103

Merged
merged 9 commits into from
Jan 4, 2025
Merged

Add support for AggregateError #103

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
49c7a4f
Minimum Node 18
fregante Jan 3, 2025
f2a0023
Add failing test
fregante Jan 3, 2025
88b3e4d
Fix
fregante Jan 3, 2025
3c28d87
Merge remote-tracking branch 'upstream/main' into enumerable
fregante Jan 3, 2025
174290c
Add support for AggregateError
fregante Jan 4, 2025
5b064d0
Merge remote-tracking branch 'upstream/main' into aggregateerror
fregante Jan 4, 2025
8b53477
Merge remote-tracking branch 'upstream/main' into aggregateerror
fregante Jan 4, 2025
a8199c7
Update index.js
sindresorhus Jan 4, 2025
5ceb3a5
Merge remote-tracking branch 'upstream/main' into aggregateerror
fregante Jan 4, 2025
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
1 change: 1 addition & 0 deletions error-constructors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const list = [
SyntaxError,
TypeError,
URIError,
AggregateError,

// Built-in errors
globalThis.DOMException,
Expand Down
21 changes: 15 additions & 6 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ const errorProperties = [
property: 'cause',
enumerable: false,
},
{
property: 'errors',
enumerable: false,
},
];

const toJsonWasCalled = new WeakSet();
Expand All @@ -48,7 +52,12 @@ const toJSON = from => {
return json;
};

const getErrorConstructor = name => errorConstructors.get(name) ?? Error;
const newError = name => {
const ErrorConstructor = errorConstructors.get(name) ?? Error;
return ErrorConstructor === globalThis.AggregateError
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
? new ErrorConstructor([])
: new ErrorConstructor();
};

// eslint-disable-next-line complexity
const destroyCircular = ({
Expand All @@ -65,8 +74,7 @@ const destroyCircular = ({
if (Array.isArray(from)) {
to = [];
} else if (!serialize && isErrorLike(from)) {
const Error = getErrorConstructor(from.name);
to = new Error();
to = newError(from.name);
} else {
to = {};
}
Expand Down Expand Up @@ -131,7 +139,9 @@ const destroyCircular = ({
for (const {property, enumerable} of errorProperties) {
if (from[property] !== undefined && from[property] !== null) {
Object.defineProperty(to, property, {
value: isErrorLike(from[property]) ? continueDestroyCircular(from[property]) : from[property],
value: isErrorLike(from[property]) || Array.isArray(from[property])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
value: isErrorLike(from[property]) || Array.isArray(from[property])
value: property === 'cause' || property === 'errors'

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a potentially breaking change though. And not directly related to this pull request. I would prefer it to be a separate pull request and also, cause is (unfortunately) not required to be an error. There should be a test for that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it breaks when cause is not an object.

Merge as is

? continueDestroyCircular(from[property])
: from[property],
enumerable: forceEnumerable ? true : enumerable,
configurable: true,
writable: true,
Expand Down Expand Up @@ -179,11 +189,10 @@ export function deserializeError(value, options = {}) {
}

if (isMinimumViableSerializedError(value)) {
const Error = getErrorConstructor(value.name);
return destroyCircular({
from: value,
seen: [],
to: new Error(),
to: newError(value.name),
maxDepth,
depth: 0,
serialize: false,
Expand Down
29 changes: 29 additions & 0 deletions test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,20 @@ test('should serialize the cause property', t => {
t.false(serialized.cause.cause instanceof Error);
});

test('should serialize AggregateError', t => {
// eslint-disable-next-line unicorn/error-message -- Testing this eventuality
const error = new AggregateError([new Error('inner error')]);

const serialized = serializeError(error);
t.is(serialized.message, ''); // Default error message
t.true(Array.isArray(serialized.errors));
t.like(serialized.errors[0], {
name: 'Error',
message: 'inner error',
});
t.false(serialized.errors[0] instanceof Error);
});

test('should handle top-level null values', t => {
const serialized = serializeError(null);
t.is(serialized, null);
Expand Down Expand Up @@ -356,6 +370,21 @@ test('should deserialize properties up to `Options.maxDepth` levels deep', t =>
t.deepEqual(levelThree, error);
});

test('should deserialize AggregateError', t => {
const deserialized = deserializeError({
name: 'AggregateError',
message: '',
errors: [
{name: 'Error', message: 'inner error', stack: ''},
],
});
t.true(deserialized instanceof AggregateError);
t.is(deserialized.message, '');
t.true(Array.isArray(deserialized.errors));
t.is(deserialized.errors[0].message, 'inner error');
t.true(deserialized.errors[0] instanceof Error);
});

test('should ignore invalid error-like objects', t => {
const errorLike = {
name: 'Error',
Expand Down
Loading