From 7918713dc57cdd8f20432c9a45071398ec577c9c Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 27 Feb 2024 13:46:24 -0800 Subject: [PATCH 1/4] only mark aggregate errors as exception groups --- packages/utils/src/aggregate-errors.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/utils/src/aggregate-errors.ts b/packages/utils/src/aggregate-errors.ts index 4547203b4fd2..864956ad4716 100644 --- a/packages/utils/src/aggregate-errors.ts +++ b/packages/utils/src/aggregate-errors.ts @@ -57,6 +57,7 @@ function aggregateExceptionsFromError( let newExceptions = [...prevExceptions]; + // Recursively call this function in order to walk down a chain of errors if (isInstanceOf(error[key], Error)) { applyExceptionGroupFieldsForParentException(exception, exceptionId); const newException = exceptionFromErrorImplementation(parser, error[key]); @@ -106,7 +107,7 @@ function applyExceptionGroupFieldsForParentException(exception: Exception, excep exception.mechanism = { ...exception.mechanism, - is_exception_group: true, + ...(exception.type === 'AggregateError' && { is_exception_group: true }), exception_id: exceptionId, }; } From 1305d0f1bcf9c5b12fda15bffa94eb6b0fea0fb0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 27 Feb 2024 15:12:21 -0800 Subject: [PATCH 2/4] make `exceptionFromError` include `type` in returned exception value --- packages/utils/test/aggregate-errors.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/utils/test/aggregate-errors.test.ts b/packages/utils/test/aggregate-errors.test.ts index 66b0f3fcfdb1..b9821166eca3 100644 --- a/packages/utils/test/aggregate-errors.test.ts +++ b/packages/utils/test/aggregate-errors.test.ts @@ -4,7 +4,7 @@ import { applyAggregateErrorsToEvent, createStackParser } from '../src/index'; const stackParser = createStackParser([0, line => ({ filename: line })]); const exceptionFromError = (_stackParser: StackParser, ex: Error): Exception => { - return { value: ex.message, mechanism: { type: 'instrument', handled: true } }; + return { value: ex.message, type: ex.name, mechanism: { type: 'instrument', handled: true } }; }; describe('applyAggregateErrorsToEvent()', () => { @@ -57,6 +57,7 @@ describe('applyAggregateErrorsToEvent()', () => { exception: { values: [ { + type: 'Error', value: 'Nested Error 2', mechanism: { exception_id: 2, @@ -67,6 +68,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Nested Error 1', mechanism: { exception_id: 1, @@ -78,6 +80,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Root Error', mechanism: { exception_id: 0, @@ -123,6 +126,7 @@ describe('applyAggregateErrorsToEvent()', () => { // Last exception in list should be the root exception expect(event.exception?.values?.[event.exception?.values.length - 1]).toStrictEqual({ + type: 'Error', value: 'Root Error', mechanism: { exception_id: 0, @@ -167,6 +171,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'cause', type: 'chained', }, + type: 'Error', value: 'Nested Error 4', }, { @@ -178,6 +183,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'errors[1]', type: 'chained', }, + type: 'Error', value: 'Nested Error 3', }, { @@ -188,6 +194,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'errors[0]', type: 'chained', }, + type: 'Error', value: 'Nested Error 2', }, { @@ -199,6 +206,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'errors[1]', type: 'chained', }, + type: 'Error', value: 'AggregateError2', }, { @@ -209,6 +217,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'errors[0]', type: 'chained', }, + type: 'Error', value: 'Nested Error 1', }, { @@ -218,6 +227,7 @@ describe('applyAggregateErrorsToEvent()', () => { is_exception_group: true, type: 'instrument', }, + type: 'Error', value: 'AggregateError1', }, ], @@ -239,6 +249,7 @@ describe('applyAggregateErrorsToEvent()', () => { exception: { values: [ { + type: 'Error', value: 'Nested Error 2', mechanism: { exception_id: 2, @@ -249,6 +260,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Nested Error 1', mechanism: { exception_id: 1, @@ -260,6 +272,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Root Error', mechanism: { exception_id: 0, @@ -287,6 +300,7 @@ describe('applyAggregateErrorsToEvent()', () => { exception: { values: [ { + type: 'Error', value: 'Nested Error 2 ...', mechanism: { exception_id: 2, @@ -297,6 +311,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Nested Error 1 ...', mechanism: { exception_id: 1, @@ -308,6 +323,7 @@ describe('applyAggregateErrorsToEvent()', () => { }, }, { + type: 'Error', value: 'Root Error with...', mechanism: { exception_id: 0, From 1b4afe13ff1b8508f558191f50dee4acde1d695d Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 27 Feb 2024 15:25:22 -0800 Subject: [PATCH 3/4] create `FakeAggregateError` class --- packages/utils/test/aggregate-errors.test.ts | 29 ++++++++++++++------ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/packages/utils/test/aggregate-errors.test.ts b/packages/utils/test/aggregate-errors.test.ts index b9821166eca3..2648a43c09e6 100644 --- a/packages/utils/test/aggregate-errors.test.ts +++ b/packages/utils/test/aggregate-errors.test.ts @@ -6,6 +6,15 @@ const stackParser = createStackParser([0, line => ({ filename: line })]); const exceptionFromError = (_stackParser: StackParser, ex: Error): Exception => { return { value: ex.message, type: ex.name, mechanism: { type: 'instrument', handled: true } }; }; +class FakeAggregateError extends Error { + public errors: Error[]; + + constructor(errors: Error[], message: string) { + super(message); + this.errors = errors; + this.name = 'AggregateError'; + } +} describe('applyAggregateErrorsToEvent()', () => { test('should not do anything if event does not contain an exception', () => { @@ -138,8 +147,10 @@ describe('applyAggregateErrorsToEvent()', () => { }); test('should keep the original mechanism type for the root exception', () => { - const fakeAggregateError: ExtendedError = new Error('Root Error'); - fakeAggregateError.errors = [new Error('Nested Error 1'), new Error('Nested Error 2')]; + const fakeAggregateError = new FakeAggregateError( + [new Error('Nested Error 1'), new Error('Nested Error 2')], + 'Root Error', + ); const event: Event = { exception: { values: [exceptionFromError(stackParser, fakeAggregateError)] } }; const eventHint: EventHint = { originalException: fakeAggregateError }; @@ -151,10 +162,12 @@ describe('applyAggregateErrorsToEvent()', () => { test('should recursively walk mixed errors (Aggregate errors and based on `key`)', () => { const chainedError: ExtendedError = new Error('Nested Error 3'); chainedError.cause = new Error('Nested Error 4'); - const fakeAggregateError2: ExtendedError = new Error('AggregateError2'); - fakeAggregateError2.errors = [new Error('Nested Error 2'), chainedError]; - const fakeAggregateError1: ExtendedError = new Error('AggregateError1'); - fakeAggregateError1.errors = [new Error('Nested Error 1'), fakeAggregateError2]; + + const fakeAggregateError2 = new FakeAggregateError([new Error('Nested Error 2'), chainedError], 'AggregateError2'); + const fakeAggregateError1 = new FakeAggregateError( + [new Error('Nested Error 1'), fakeAggregateError2], + 'AggregateError1', + ); const event: Event = { exception: { values: [exceptionFromError(stackParser, fakeAggregateError1)] } }; const eventHint: EventHint = { originalException: fakeAggregateError1 }; @@ -206,7 +219,7 @@ describe('applyAggregateErrorsToEvent()', () => { source: 'errors[1]', type: 'chained', }, - type: 'Error', + type: 'AggregateError', value: 'AggregateError2', }, { @@ -227,7 +240,7 @@ describe('applyAggregateErrorsToEvent()', () => { is_exception_group: true, type: 'instrument', }, - type: 'Error', + type: 'AggregateError', value: 'AggregateError1', }, ], From 16b90d935625bf74ce093c6ca04eea404cc53685 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 27 Feb 2024 15:27:10 -0800 Subject: [PATCH 4/4] remove stray `is_exception_group` properties on non-aggregate errors --- packages/utils/test/aggregate-errors.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/utils/test/aggregate-errors.test.ts b/packages/utils/test/aggregate-errors.test.ts index 2648a43c09e6..8d5fb3be6ded 100644 --- a/packages/utils/test/aggregate-errors.test.ts +++ b/packages/utils/test/aggregate-errors.test.ts @@ -83,7 +83,6 @@ describe('applyAggregateErrorsToEvent()', () => { exception_id: 1, handled: true, parent_id: 0, - is_exception_group: true, source: 'cause', type: 'chained', }, @@ -94,7 +93,6 @@ describe('applyAggregateErrorsToEvent()', () => { mechanism: { exception_id: 0, handled: true, - is_exception_group: true, type: 'instrument', }, }, @@ -140,7 +138,6 @@ describe('applyAggregateErrorsToEvent()', () => { mechanism: { exception_id: 0, handled: true, - is_exception_group: true, type: 'instrument', }, }); @@ -191,7 +188,6 @@ describe('applyAggregateErrorsToEvent()', () => { mechanism: { exception_id: 4, handled: true, - is_exception_group: true, parent_id: 2, source: 'errors[1]', type: 'chained', @@ -279,7 +275,6 @@ describe('applyAggregateErrorsToEvent()', () => { exception_id: 1, handled: true, parent_id: 0, - is_exception_group: true, source: 'cause', type: 'chained', }, @@ -290,7 +285,6 @@ describe('applyAggregateErrorsToEvent()', () => { mechanism: { exception_id: 0, handled: true, - is_exception_group: true, type: 'instrument', }, }, @@ -330,7 +324,6 @@ describe('applyAggregateErrorsToEvent()', () => { exception_id: 1, handled: true, parent_id: 0, - is_exception_group: true, source: 'cause', type: 'chained', }, @@ -341,7 +334,6 @@ describe('applyAggregateErrorsToEvent()', () => { mechanism: { exception_id: 0, handled: true, - is_exception_group: true, type: 'instrument', }, },