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

Aggregate exceptions' `exception.value` strings weren't truncated correctly in our event preparation pipeline. The tricky part here is that the `LinkedErrors` integration adds the linked/child exceptions to the event in an event processor which is applied only after we truncate the original event. Hence, we need to also truncate the values in the event processor itself.
getsentry · Jul 21, 2023 · 7d7b8ad · 7d7b8ad
1 parent da07542
commit 7d7b8ad
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 21 deletions.
diff --git a/...ges/browser-integration-tests/suites/public-api/captureException/linkedErrrors/subject.js b/...ges/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
@@ -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),
+    },
+  });
+});
diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts
@@ -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,

diff --git 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
@@ -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,
@@ -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,
     );
   }
 }
@@ -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;
+  });
+}
diff --git 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,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: [] } });
@@ -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',
+            },
+          },
+        ],
+      },
+    });
+  });
 });