From e90834a6f63ced247f731d3a34be3eaafa021f7d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 19 Jul 2023 17:55:20 +0200 Subject: [PATCH 1/2] fix(utils): Truncate aggregate exception values (LinkedErrors) --- .../captureException/linkedErrrors/subject.js | 13 ++++ .../captureException/linkedErrrors/test.ts | 41 +++++++++++ .../browser/src/integrations/linkederrors.ts | 1 + .../node/src/integrations/linkederrors.ts | 5 +- packages/utils/src/aggregate-errors.ts | 37 +++++++--- packages/utils/test/aggregate-errors.test.ts | 68 ++++++++++++++++--- 6 files changed, 144 insertions(+), 21 deletions(-) create mode 100644 packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/subject.js create mode 100644 packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/test.ts diff --git a/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/subject.js b/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/subject.js new file mode 100644 index 000000000000..f7eb81412b13 --- /dev/null +++ b/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/subject.js @@ -0,0 +1,13 @@ +const wat = new Error(`This is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be`); + +wat.cause = new Error(`This is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be,`); + +Sentry.captureException(wat); diff --git a/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/test.ts b/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/test.ts new file mode 100644 index 000000000000..b78ae5e12525 --- /dev/null +++ b/packages/browser-integration-tests/suites/public-api/captureException/linkedErrrors/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; + +sentryTest('should capture a linked error with messages', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(2); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: `This is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be, +this is a very long message that should be truncated and hopefully will be, +this is a very long me...`, + mechanism: { + type: 'chained', + handled: true, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); + expect(eventData.exception?.values?.[1]).toMatchObject({ + type: 'Error', + value: `This is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be, +this is a very long message that should be truncated and will be, +this is a very long message that should be truncated...`, + mechanism: { + type: 'generic', + handled: true, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index 08581fecb9b3..a27c71369638 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -57,6 +57,7 @@ export class LinkedErrors implements Integration { applyAggregateErrorsToEvent( exceptionFromError, client.getOptions().stackParser, + client.getOptions().maxValueLength, self._key, self._limit, event, diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index d6a130ce5d54..610a376f640a 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -50,9 +50,12 @@ export class LinkedErrors implements Integration { return event; } + const options = client.getOptions(); + applyAggregateErrorsToEvent( exceptionFromError, - client.getOptions().stackParser, + options.stackParser, + options.maxValueLength, self._key, self._limit, event, diff --git a/packages/utils/src/aggregate-errors.ts b/packages/utils/src/aggregate-errors.ts index 1dcf9b1628ef..d6f2b7957851 100644 --- a/packages/utils/src/aggregate-errors.ts +++ b/packages/utils/src/aggregate-errors.ts @@ -1,6 +1,7 @@ import type { Event, EventHint, Exception, ExtendedError, StackParser } from '@sentry/types'; import { isInstanceOf } from './is'; +import { truncate } from './string'; /** * Creates exceptions inside `event.exception.values` for errors that are nested on properties based on the `key` parameter. @@ -8,6 +9,7 @@ import { isInstanceOf } from './is'; export function applyAggregateErrorsToEvent( exceptionFromErrorImplementation: (stackParser: StackParser, ex: Error) => Exception, parser: StackParser, + maxValueLimit: number = 250, key: string, limit: number, event: Event, @@ -23,16 +25,17 @@ export function applyAggregateErrorsToEvent( // We only create exception grouping if there is an exception in the event. if (originalException) { - event.exception.values = aggregateExceptionsFromError( - exceptionFromErrorImplementation, - parser, - limit, - hint.originalException as ExtendedError, - key, - event.exception.values, - originalException, - 0, - ); + event.exception.values = truncateAggregateExceptions( + aggregateExceptionsFromError( + exceptionFromErrorImplementation, + parser, + limit, + hint.originalException as ExtendedError, + key, + event.exception.values, + originalException, + 0, + ), maxValueLimit); } } @@ -123,3 +126,17 @@ function applyExceptionGroupFieldsForChildException( parent_id: parentId, }; } + +/** + * Truncate the message (exception.value) of all exceptions in the event. + * Because this event processor is ran after `applyClientOptions`, + * we need to truncate the message of the added exceptions here. + */ +function truncateAggregateExceptions(exceptions: Exception[], maxValueLength: number): Exception[] { + return exceptions.map(exception => { + if (exception.value) { + exception.value = truncate(exception.value, maxValueLength); + } + return exception; + }); +} diff --git a/packages/utils/test/aggregate-errors.test.ts b/packages/utils/test/aggregate-errors.test.ts index 9a42bba12858..66b0f3fcfdb1 100644 --- a/packages/utils/test/aggregate-errors.test.ts +++ b/packages/utils/test/aggregate-errors.test.ts @@ -11,7 +11,7 @@ describe('applyAggregateErrorsToEvent()', () => { test('should not do anything if event does not contain an exception', () => { const event: Event = { exception: undefined }; const eventHint: EventHint = { originalException: new Error() }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); // no changes expect(event).toStrictEqual({ exception: undefined }); @@ -20,7 +20,7 @@ describe('applyAggregateErrorsToEvent()', () => { test('should not do anything if event does not contain exception values', () => { const event: Event = { exception: { values: undefined } }; const eventHint: EventHint = { originalException: new Error() }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); // no changes expect(event).toStrictEqual({ exception: { values: undefined } }); @@ -28,7 +28,7 @@ describe('applyAggregateErrorsToEvent()', () => { test('should not do anything if event does not contain an event hint', () => { const event: Event = { exception: { values: [] } }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, undefined); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, undefined); // no changes expect(event).toStrictEqual({ exception: { values: [] } }); @@ -37,7 +37,7 @@ describe('applyAggregateErrorsToEvent()', () => { test('should not do anything if the event hint does not contain an original exception', () => { const event: Event = { exception: { values: [] } }; const eventHint: EventHint = { originalException: undefined }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); // no changes expect(event).toStrictEqual({ exception: { values: [] } }); @@ -52,7 +52,7 @@ describe('applyAggregateErrorsToEvent()', () => { const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } }; const eventHint: EventHint = { originalException }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, key, 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, key, 100, event, eventHint); expect(event).toStrictEqual({ exception: { values: [ @@ -97,7 +97,7 @@ describe('applyAggregateErrorsToEvent()', () => { const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } }; const eventHint: EventHint = { originalException }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); // no changes expect(event).toStrictEqual({ exception: { values: [exceptionFromError(stackParser, originalException)] } }); @@ -116,7 +116,7 @@ describe('applyAggregateErrorsToEvent()', () => { } const eventHint: EventHint = { originalException }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, key, 5, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, key, 5, event, eventHint); // 6 -> one for original exception + 5 linked expect(event.exception?.values).toHaveLength(5 + 1); @@ -140,7 +140,7 @@ describe('applyAggregateErrorsToEvent()', () => { const event: Event = { exception: { values: [exceptionFromError(stackParser, fakeAggregateError)] } }; const eventHint: EventHint = { originalException: fakeAggregateError }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); expect(event.exception?.values?.[event.exception.values.length - 1].mechanism?.type).toBe('instrument'); }); @@ -155,7 +155,7 @@ describe('applyAggregateErrorsToEvent()', () => { const event: Event = { exception: { values: [exceptionFromError(stackParser, fakeAggregateError1)] } }; const eventHint: EventHint = { originalException: fakeAggregateError1 }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, 'cause', 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, 'cause', 100, event, eventHint); expect(event).toStrictEqual({ exception: { values: [ @@ -234,7 +234,7 @@ describe('applyAggregateErrorsToEvent()', () => { const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } }; const eventHint: EventHint = { originalException }; - applyAggregateErrorsToEvent(exceptionFromError, stackParser, key, 100, event, eventHint); + applyAggregateErrorsToEvent(exceptionFromError, stackParser, undefined, key, 100, event, eventHint); expect(event).toStrictEqual({ exception: { values: [ @@ -272,4 +272,52 @@ describe('applyAggregateErrorsToEvent()', () => { }, }); }); + + test('should truncate the exception values if they exceed the `maxValueLength` option', () => { + const originalException: ExtendedError = new Error('Root Error with long message'); + originalException.cause = new Error('Nested Error 1 with longer message'); + originalException.cause.cause = new Error('Nested Error 2 with longer message with longer message'); + + const event: Event = { exception: { values: [exceptionFromError(stackParser, originalException)] } }; + const eventHint: EventHint = { originalException }; + + const maxValueLength = 15; + applyAggregateErrorsToEvent(exceptionFromError, stackParser, maxValueLength, 'cause', 10, event, eventHint); + expect(event).toStrictEqual({ + exception: { + values: [ + { + value: 'Nested Error 2 ...', + mechanism: { + exception_id: 2, + handled: true, + parent_id: 1, + source: 'cause', + type: 'chained', + }, + }, + { + value: 'Nested Error 1 ...', + mechanism: { + exception_id: 1, + handled: true, + parent_id: 0, + is_exception_group: true, + source: 'cause', + type: 'chained', + }, + }, + { + value: 'Root Error with...', + mechanism: { + exception_id: 0, + handled: true, + is_exception_group: true, + type: 'instrument', + }, + }, + ], + }, + }); + }); }); From 21914c0eb19ab9bfee6b0d65fe8e9876154f9d2c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 20 Jul 2023 11:32:13 +0200 Subject: [PATCH 2/2] cleanup --- packages/browser/src/integrations/linkederrors.ts | 5 +++-- packages/utils/src/aggregate-errors.ts | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index a27c71369638..709ba8228107 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -54,10 +54,11 @@ export class LinkedErrors implements Integration { return event; } + const options = client.getOptions(); applyAggregateErrorsToEvent( exceptionFromError, - client.getOptions().stackParser, - client.getOptions().maxValueLength, + options.stackParser, + options.maxValueLength, self._key, self._limit, event, diff --git a/packages/utils/src/aggregate-errors.ts b/packages/utils/src/aggregate-errors.ts index d6f2b7957851..4547203b4fd2 100644 --- a/packages/utils/src/aggregate-errors.ts +++ b/packages/utils/src/aggregate-errors.ts @@ -35,7 +35,9 @@ export function applyAggregateErrorsToEvent( event.exception.values, originalException, 0, - ), maxValueLimit); + ), + maxValueLimit, + ); } }