Skip to content

Commit

Permalink
fix(utils): Truncate aggregate exception values (LinkedErrors)
Browse files Browse the repository at this point in the history
  • Loading branch information
Lms24 committed Jul 19, 2023
1 parent 1c28688 commit e90834a
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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<Event>(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),
},
});
});
1 change: 1 addition & 0 deletions packages/browser/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export class LinkedErrors implements Integration {
applyAggregateErrorsToEvent(
exceptionFromError,
client.getOptions().stackParser,
client.getOptions().maxValueLength,
self._key,
self._limit,
event,
Expand Down
5 changes: 4 additions & 1 deletion packages/node/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 27 additions & 10 deletions packages/utils/src/aggregate-errors.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
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.
*/
export function applyAggregateErrorsToEvent(
exceptionFromErrorImplementation: (stackParser: StackParser, ex: Error) => Exception,
parser: StackParser,
maxValueLimit: number = 250,
key: string,
limit: number,
event: Event,
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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;
});
}
68 changes: 58 additions & 10 deletions packages/utils/test/aggregate-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -20,15 +20,15 @@ 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 } });
});

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: [] } });
Expand All @@ -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: [] } });
Expand All @@ -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: [
Expand Down Expand Up @@ -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)] } });
Expand All @@ -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);
Expand All @@ -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');
});

Expand All @@ -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: [
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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',
},
},
],
},
});
});
});

0 comments on commit e90834a

Please sign in to comment.