Repository navigation
feat(core): Support SuppressedError in LinkedErrors integration #25246
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.
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,18 @@ class CustomAggregateError extends FakeAggregateError { | |
| } | ||
| } | ||
|
|
||
| class FakeSuppressedError extends Error { | ||
| public error: unknown; | ||
| public suppressed: unknown; | ||
|
|
||
| constructor(error: unknown, suppressed: unknown, message = 'An error was suppressed during disposal.') { | ||
| super(message); | ||
| this.name = 'SuppressedError'; | ||
| this.error = error; | ||
| this.suppressed = suppressed; | ||
| } | ||
| } | ||
|
|
||
| describe('applyAggregateErrorsToEvent()', () => { | ||
| test('should not do anything if event does not contain an exception', () => { | ||
| const event: Event = { exception: undefined }; | ||
|
|
@@ -482,4 +494,76 @@ describe('applyAggregateErrorsToEvent()', () => { | |
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test('adds both the `error` and the `suppressed` error of a SuppressedError', () => { | ||
| const suppressedError = new FakeSuppressedError(new Error('Error from dispose'), new Error('Original error')); | ||
| const event: Event = { exception: { values: [exceptionFromError(stackParser, suppressedError)] } }; | ||
| const eventHint: EventHint = { originalException: suppressedError }; | ||
|
|
||
| applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); | ||
|
|
||
| expect(event).toStrictEqual({ | ||
| exception: { | ||
| values: [ | ||
| { | ||
| mechanism: { exception_id: 2, handled: true, parent_id: 0, source: 'suppressed', type: 'chained' }, | ||
| type: 'Error', | ||
| value: 'Original error', | ||
| }, | ||
| { | ||
| mechanism: { exception_id: 1, handled: true, parent_id: 0, source: 'error', type: 'chained' }, | ||
| type: 'Error', | ||
| value: 'Error from dispose', | ||
| }, | ||
| { | ||
| mechanism: { exception_id: 0, handled: true, type: 'instrument' }, | ||
| type: 'SuppressedError', | ||
| value: 'An error was suppressed during disposal.', | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| test('walks nested SuppressedErrors (several disposers throwing)', () => { | ||
| const inner = new FakeSuppressedError(new Error('dispose 2 failed'), new Error('Original error')); | ||
| const outer = new FakeSuppressedError(new Error('dispose 1 failed'), inner); | ||
| const event: Event = { exception: { values: [exceptionFromError(stackParser, outer)] } }; | ||
|
|
||
| applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, { originalException: outer }); | ||
|
|
||
| expect(event.exception?.values?.map(e => e.value)).toEqual([ | ||
| 'Original error', | ||
| 'dispose 2 failed', | ||
| 'An error was suppressed during disposal.', | ||
| 'dispose 1 failed', | ||
| 'An error was suppressed during disposal.', | ||
| ]); | ||
| }); | ||
|
|
||
| test('skips `error` / `suppressed` values that are not Errors', () => { | ||
| const suppressedError = new FakeSuppressedError('just a string', new Error('Original error')); | ||
| const event: Event = { exception: { values: [exceptionFromError(stackParser, suppressedError)] } }; | ||
|
|
||
| applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, { | ||
| originalException: suppressedError, | ||
| }); | ||
|
|
||
| expect(event.exception?.values?.map(e => e.value)).toEqual([ | ||
| 'Original error', | ||
| 'An error was suppressed during disposal.', | ||
| ]); | ||
| }); | ||
|
|
||
| test('does not treat an error that only has an `error` property as a SuppressedError', () => { | ||
| const notSuppressed: ExtendedError = new Error('Something else'); | ||
| notSuppressed.error = new Error('unrelated'); | ||
| const event: Event = { exception: { values: [exceptionFromError(stackParser, notSuppressed)] } }; | ||
|
|
||
| applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, { | ||
| originalException: notSuppressed, | ||
| }); | ||
|
|
||
| expect(event.exception?.values).toHaveLength(1); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feat lacks integration coverageMedium Severity This feat only adds unit tests around Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit 1834d47. Configure here. |
||
| }); | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feat is missing an integration test
Medium Severity
Flagged because the review rules require feat PRs to include an integration or E2E test. Coverage here is unit-only via
FakeSuppressedError, so nativeusing/SuppressedErrornever runs through LinkedErrors. A Node test gated withtest.runIf(supports({ min: 22 }))would close that gap.Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 91c0b88. Configure here.