Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class CustomAggregateError extends AggregateError {
Copy link
Member Author

Choose a reason for hiding this comment

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

this test primarily demonstrates that this works for errors with the actual builtin AggregateError class

constructor(errors, message, options) {
super(errors, message, options);
this.name = 'CustomAggregateError';
}
}

const aggregateError = new CustomAggregateError(
[new Error('error 1', { cause: new Error('error 1 cause') }), new Error('error 2')],
'custom aggregate error',
{
cause: new Error('aggregate cause'),
},
);

Sentry.captureException(aggregateError);
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';

sentryTest('captures custom AggregateErrors', async ({ getLocalTestUrl, page }) => {
const url = await getLocalTestUrl({ testDir: __dirname });
const req = await waitForErrorRequestOnUrl(page, url);
const eventData = envelopeRequestParser(req);

expect(eventData.exception?.values).toHaveLength(5); // CustomAggregateError + 3 embedded errors + 1 aggregate cause

// Verify the embedded errors come first
expect(eventData.exception?.values).toEqual([
expect.objectContaining({
mechanism: { exception_id: 4, handled: true, parent_id: 0, source: 'errors[1]', type: 'chained' },
type: 'Error',
value: 'error 2',
}),
expect.objectContaining({
mechanism: { exception_id: 3, handled: true, parent_id: 2, source: 'cause', type: 'chained' },
type: 'Error',
value: 'error 1 cause',
}),
expect.objectContaining({
mechanism: { exception_id: 2, handled: true, parent_id: 0, source: 'errors[0]', type: 'chained' },
type: 'Error',
value: 'error 1',
}),
expect.objectContaining({
mechanism: { exception_id: 1, handled: true, parent_id: 0, source: 'cause', type: 'chained' },
type: 'Error',
value: 'aggregate cause',
}),
expect.objectContaining({
mechanism: { exception_id: 0, handled: true, type: 'generic', is_exception_group: true },
type: 'CustomAggregateError',
value: 'custom aggregate error',
}),
]);
});
18 changes: 13 additions & 5 deletions packages/core/src/utils/aggregate-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function aggregateExceptionsFromError(

// Recursively call this function in order to walk down a chain of errors
if (isInstanceOf(error[key], Error)) {
applyExceptionGroupFieldsForParentException(exception, exceptionId);
applyExceptionGroupFieldsForParentException(exception, exceptionId, error);
const newException = exceptionFromErrorImplementation(parser, error[key] as Error);
const newExceptionId = newExceptions.length;
applyExceptionGroupFieldsForChildException(newException, key, newExceptionId, exceptionId);
Expand All @@ -74,10 +74,10 @@ function aggregateExceptionsFromError(

// This will create exception grouping for AggregateErrors
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/AggregateError
if (Array.isArray(error.errors)) {
if (isExceptionGroup(error)) {
error.errors.forEach((childError, i) => {
if (isInstanceOf(childError, Error)) {
applyExceptionGroupFieldsForParentException(exception, exceptionId);
applyExceptionGroupFieldsForParentException(exception, exceptionId, error);
const newException = exceptionFromErrorImplementation(parser, childError as Error);
const newExceptionId = newExceptions.length;
applyExceptionGroupFieldsForChildException(newException, `errors[${i}]`, newExceptionId, exceptionId);
Expand All @@ -98,12 +98,20 @@ function aggregateExceptionsFromError(
return newExceptions;
}

function applyExceptionGroupFieldsForParentException(exception: Exception, exceptionId: number): void {
function isExceptionGroup(error: ExtendedError): error is ExtendedError & { errors: unknown[] } {
return Array.isArray(error.errors);
}

function applyExceptionGroupFieldsForParentException(
exception: Exception,
exceptionId: number,
error: ExtendedError,
): void {
exception.mechanism = {
handled: true,
type: 'auto.core.linked_errors',
...(isExceptionGroup(error) && { is_exception_group: true }),
Copy link

Choose a reason for hiding this comment

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

Order change allows mechanism to override is_exception_group

Low Severity

The spread order in applyExceptionGroupFieldsForParentException changed from setting is_exception_group: true AFTER ...exception.mechanism to BEFORE it. Previously, the flag would always override for exception groups. Now, if exception.mechanism already contains an is_exception_group value (even false), it will override the duck-typed detection. This could cause exception groups to be incorrectly classified if a custom exceptionFromErrorImplementation sets is_exception_group in the mechanism.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah but I think this is actually more correct. Theoretically users could set this beforehand and I don't think it hurts us to flip this in the default case. Gonna leave it as-is.

...exception.mechanism,
...(exception.type === 'AggregateError' && { is_exception_group: true }),
exception_id: exceptionId,
};
}
Expand Down
62 changes: 62 additions & 0 deletions packages/core/test/lib/utils/aggregate-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,16 @@ class FakeAggregateError extends Error {
}
}

class CustomAggregateError extends FakeAggregateError {
public cause?: Error;

constructor(errors: Error[], message: string, cause?: Error) {
super(errors, message);
this.name = 'CustomAggregateError';
this.cause = cause;
}
}

describe('applyAggregateErrorsToEvent()', () => {
test('should not do anything if event does not contain an exception', () => {
const event: Event = { exception: undefined };
Expand Down Expand Up @@ -316,4 +326,56 @@ describe('applyAggregateErrorsToEvent()', () => {
},
});
});

test('marks custom AggregateErrors as exception groups', () => {
const customAggregateError = new CustomAggregateError(
[new Error('Nested Error 1')],
'my CustomAggregateError',
new Error('Aggregate Cause'),
);

const event: Event = { exception: { values: [exceptionFromError(stackParser, customAggregateError)] } };
const eventHint: EventHint = { originalException: customAggregateError };

applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint);

expect(event).toStrictEqual({
exception: {
values: [
{
mechanism: {
exception_id: 2,
handled: true,
parent_id: 0,
source: 'errors[0]',
type: 'chained',
},
type: 'Error',
value: 'Nested Error 1',
},
{
mechanism: {
exception_id: 1,
handled: true,
parent_id: 0,
source: 'cause',
type: 'chained',
},
type: 'Error',
value: 'Aggregate Cause',
},
{
mechanism: {
exception_id: 0,
handled: true,
type: 'instrument',
is_exception_group: true,
},
type: 'CustomAggregateError',
value: 'my CustomAggregateError',
},
],
},
});
});
});
Loading