fix(core): Use maxValueLength in extra error data integration (#12174)

fixes #12168 We should truncate extra error data keys using `maxValueLength` to make sure event payload size stays reasonably small.
getsentry · Jun 19, 2024 · 2fbd7cc · 2fbd7cc
1 parent 1d66b8a
commit 2fbd7cc
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 18 deletions.
diff --git a/packages/core/src/integrations/extraerrordata.ts b/packages/core/src/integrations/extraerrordata.ts
@@ -1,5 +1,5 @@
 import type { Contexts, Event, EventHint, ExtendedError, IntegrationFn } from '@sentry/types';
-import { addNonEnumerableProperty, isError, isPlainObject, logger, normalize } from '@sentry/utils';
+import { addNonEnumerableProperty, isError, isPlainObject, logger, normalize, truncate } from '@sentry/utils';
 import { defineIntegration } from '../integration';
 
 import { DEBUG_BUILD } from '../debug-build';
@@ -27,8 +27,9 @@ const _extraErrorDataIntegration = ((options: Partial<ExtraErrorDataOptions> = {
   const { depth = 3, captureErrorCause = true } = options;
   return {
     name: INTEGRATION_NAME,
-    processEvent(event, hint) {
-      return _enhanceEventWithErrorData(event, hint, depth, captureErrorCause);
+    processEvent(event, hint, client) {
+      const { maxValueLength = 250 } = client.getOptions();
+      return _enhanceEventWithErrorData(event, hint, depth, captureErrorCause, maxValueLength);
     },
   };
 }) satisfies IntegrationFn;
@@ -40,13 +41,14 @@ function _enhanceEventWithErrorData(
   hint: EventHint = {},
   depth: number,
   captureErrorCause: boolean,
+  maxValueLength: number,
 ): Event {
   if (!hint.originalException || !isError(hint.originalException)) {
     return event;
   }
   const exceptionName = (hint.originalException as ExtendedError).name || hint.originalException.constructor.name;
 
-  const errorData = _extractErrorData(hint.originalException as ExtendedError, captureErrorCause);
+  const errorData = _extractErrorData(hint.originalException as ExtendedError, captureErrorCause, maxValueLength);
 
   if (errorData) {
     const contexts: Contexts = {
@@ -74,7 +76,11 @@ function _enhanceEventWithErrorData(
 /**
  * Extract extra information from the Error object
  */
-function _extractErrorData(error: ExtendedError, captureErrorCause: boolean): Record<string, unknown> | null {
+function _extractErrorData(
+  error: ExtendedError,
+  captureErrorCause: boolean,
+  maxValueLength: number,
+): Record<string, unknown> | null {
   // We are trying to enhance already existing event, so no harm done if it won't succeed
   try {
     const nativeKeys = [
@@ -97,7 +103,7 @@ function _extractErrorData(error: ExtendedError, captureErrorCause: boolean): Re
         continue;
       }
       const value = error[key];
-      extraErrorInfo[key] = isError(value) ? value.toString() : value;
+      extraErrorInfo[key] = isError(value) || typeof value === 'string' ? truncate(`${value}`, maxValueLength) : value;
     }
 
     // Error.cause is a standard property that is non enumerable, we therefore need to access it separately.

diff --git a/packages/core/test/lib/integrations/extraerrordata.test.ts b/packages/core/test/lib/integrations/extraerrordata.test.ts
@@ -2,10 +2,13 @@ import type { Event as SentryEvent, ExtendedError } from '@sentry/types';
 
 import { extraErrorDataIntegration } from '../../../src/integrations/extraerrordata';
 
+import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
+
 const extraErrorData = extraErrorDataIntegration();
 let event: SentryEvent;
 
 describe('ExtraErrorData()', () => {
+  const testClient = new TestClient(getDefaultTestClientOptions({ maxValueLength: 250 }));
   beforeEach(() => {
     event = {};
   });
@@ -20,7 +23,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -31,6 +34,27 @@ describe('ExtraErrorData()', () => {
     });
   });
 
+  it('should use maxValueLength to truncate extra data', () => {
+    const error = new TypeError('foo') as ExtendedError;
+    error.baz = 42;
+    error.foo = 'a'.repeat(300);
+
+    const enhancedEvent = extraErrorData.processEvent?.(
+      event,
+      {
+        originalException: error,
+      },
+      testClient,
+    ) as SentryEvent;
+
+    expect(enhancedEvent.contexts).toEqual({
+      TypeError: {
+        baz: 42,
+        foo: `${'a'.repeat(250)}...`,
+      },
+    });
+  });
+
   it('doesnt choke on linked errors and stringify names instead', () => {
     const error = new TypeError('foo') as ExtendedError;
     error.cause = new SyntaxError('bar');
@@ -40,7 +64,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -65,7 +89,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -93,7 +117,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -112,14 +136,14 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent).toEqual(event);
   });
 
   it('should return event if there is no SentryEventHint', () => {
-    const enhancedEvent = extraErrorData.processEvent?.(event, {}, {} as any);
+    const enhancedEvent = extraErrorData.processEvent?.(event, {}, testClient);
 
     expect(enhancedEvent).toEqual(event);
   });
@@ -131,7 +155,7 @@ describe('ExtraErrorData()', () => {
         // @ts-expect-error Allow event to have extra properties
         notOriginalException: 'fooled you',
       },
-      {} as any,
+      testClient,
     );
 
     expect(enhancedEvent).toEqual(event);
@@ -153,7 +177,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -180,7 +204,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -204,7 +228,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -232,7 +256,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).toEqual({
@@ -261,7 +285,7 @@ describe('ExtraErrorData()', () => {
       {
         originalException: error,
       },
-      {} as any,
+      testClient,
     ) as SentryEvent;
 
     expect(enhancedEvent.contexts).not.toEqual({