Skip to content
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

fix(utils): Truncate aggregate exception values (LinkedErrors) #8593

Merged
merged 2 commits into from
Jul 21, 2023
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,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),
},
});
});
4 changes: 3 additions & 1 deletion packages/browser/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ 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
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: 28 additions & 9 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,15 +25,18 @@ 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 +128,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',
},
},
],
},
});
});
});