ref(core)!: Mark exceptions from captureConsoleIntegration as handled: true by default (#14734)

Lms24 · web-flow · commit 1c3f37b5a5d4 · 2024-12-16T16:52:02.000+01:00
Flip the default value for the `handled` flag in exceptions captured by `captureConsoleIntegration`. This integration only captures exceptions if `attachStackTrace` is set to `true`. Added another integration test as a follow-up as promised in #14664.
diff --git a/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/init.js b/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/init.js
@@ -0,0 +1,11 @@
+import * as Sentry from '@sentry/browser';
+import { captureConsoleIntegration } from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [captureConsoleIntegration()],
+  autoSessionTracking: false,
+  attachStacktrace: true,
+});
diff --git a/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/subject.js b/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/subject.js
@@ -0,0 +1,8 @@
+console.log('console log');
+console.warn('console warn');
+console.error('console error');
+console.info('console info');
+console.trace('console trace');
+
+console.error(new Error('console error with error object'));
+console.trace(new Error('console trace with error object'));
diff --git a/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/test.ts b/dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/test.ts
@@ -0,0 +1,176 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/core';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { getMultipleSentryEnvelopeRequests } from '../../../utils/helpers';
+
+sentryTest(
+  'captures console messages correctly and adds a synthetic stack trace if `attachStackTrace` is set to `true`',
+  async ({ getLocalTestUrl, page }) => {
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    const [, events] = await Promise.all([page.goto(url), getMultipleSentryEnvelopeRequests<Event>(page, 7)]);
+
+    expect(events).toHaveLength(7);
+
+    const logEvent = events.find(event => event.message === 'console log');
+    const warnEvent = events.find(event => event.message === 'console warn');
+    const infoEvent = events.find(event => event.message === 'console info');
+    const errorEvent = events.find(event => event.message === 'console error');
+    const traceEvent = events.find(event => event.message === 'console trace');
+    const errorWithErrorEvent = events.find(
+      event => event.exception && event.exception.values?.[0].value === 'console error with error object',
+    );
+    const traceWithErrorEvent = events.find(
+      event => event.exception && event.exception.values?.[0].value === 'console trace with error object',
+    );
+
+    expect(logEvent).toEqual(
+      expect.objectContaining({
+        level: 'log',
+        logger: 'console',
+        extra: {
+          arguments: ['console log'],
+        },
+        message: 'console log',
+      }),
+    );
+    expect(logEvent?.exception?.values![0]).toMatchObject({
+      mechanism: {
+        handled: true,
+        type: 'console',
+        synthetic: true,
+      },
+      value: 'console log',
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    expect(warnEvent).toEqual(
+      expect.objectContaining({
+        level: 'warning',
+        logger: 'console',
+        extra: {
+          arguments: ['console warn'],
+        },
+        message: 'console warn',
+      }),
+    );
+    expect(warnEvent?.exception?.values![0]).toMatchObject({
+      mechanism: {
+        handled: true,
+        type: 'console',
+        synthetic: true,
+      },
+      value: 'console warn',
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    expect(infoEvent).toEqual(
+      expect.objectContaining({
+        level: 'info',
+        logger: 'console',
+        extra: {
+          arguments: ['console info'],
+        },
+        message: 'console info',
+      }),
+    );
+    expect(infoEvent?.exception?.values![0]).toMatchObject({
+      mechanism: {
+        handled: true,
+        type: 'console',
+        synthetic: true,
+      },
+      value: 'console info',
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    expect(errorEvent).toEqual(
+      expect.objectContaining({
+        level: 'error',
+        logger: 'console',
+        extra: {
+          arguments: ['console error'],
+        },
+        message: 'console error',
+      }),
+    );
+    expect(errorEvent?.exception?.values![0]).toMatchObject({
+      mechanism: {
+        handled: true,
+        type: 'console',
+        synthetic: true,
+      },
+      value: 'console error',
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    expect(traceEvent).toEqual(
+      expect.objectContaining({
+        level: 'log',
+        logger: 'console',
+        extra: {
+          arguments: ['console trace'],
+        },
+        message: 'console trace',
+      }),
+    );
+    expect(traceEvent?.exception?.values![0]).toMatchObject({
+      mechanism: {
+        handled: true,
+        type: 'console',
+        synthetic: true,
+      },
+      value: 'console trace',
+      stacktrace: {
+        frames: expect.any(Array),
+      },
+    });
+
+    expect(errorWithErrorEvent).toEqual(
+      expect.objectContaining({
+        level: 'error',
+        logger: 'console',
+        extra: {
+          arguments: [
+            {
+              message: 'console error with error object',
+              name: 'Error',
+              stack: expect.any(String),
+            },
+          ],
+        },
+        exception: expect.any(Object),
+      }),
+    );
+    expect(errorWithErrorEvent?.exception?.values?.[0].value).toBe('console error with error object');
+    expect(errorWithErrorEvent?.exception?.values?.[0].mechanism).toEqual({
+      handled: true,
+      type: 'console',
+    });
+    expect(traceWithErrorEvent).toEqual(
+      expect.objectContaining({
+        level: 'log',
+        logger: 'console',
+        extra: {
+          arguments: [
+            {
+              message: 'console trace with error object',
+              name: 'Error',
+              stack: expect.any(String),
+            },
+          ],
+        },
+      }),
+    );
+    expect(traceWithErrorEvent?.exception?.values?.[0].value).toBe('console trace with error object');
+  },
+);
diff --git a/dev-packages/browser-integration-tests/suites/integrations/captureConsole/test.ts b/dev-packages/browser-integration-tests/suites/integrations/captureConsole/test.ts
@@ -96,8 +96,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
   );
   expect(errorWithErrorEvent?.exception?.values?.[0].value).toBe('console error with error object');
   expect(errorWithErrorEvent?.exception?.values?.[0].mechanism).toEqual({
-    // TODO (v9): Adjust to true after changing the integration's default value
-    handled: false,
+    handled: true,
     type: 'console',
   });
   expect(traceWithErrorEvent).toEqual(
diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md
@@ -78,6 +78,17 @@ If you need to support older browsers, we recommend transpiling your code using
 
 ## 2. Behavior Changes
 
+### `@sentry/core` / All SDKs
+
+- If you use the optional `captureConsoleIntegration` and set `attachStackTrace: true` in your `Sentry.init` call, console messages will no longer be marked as unhandled (i.e. `handled: false`) but as handled (i.e. `handled: true`). If you want to keep sending them as unhandled, configure the `handled` option when adding the integration:
+
+```js
+Sentry.init({
+  integrations: [Sentry.captureConsoleIntegration({ handled: false })],
+  attachStackTrace: true,
+});
+```
+
 ### `@sentry/node`
 
 - When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed.
diff --git a/packages/core/src/integrations/captureconsole.ts b/packages/core/src/integrations/captureconsole.ts
@@ -12,13 +12,11 @@ import { GLOBAL_OBJ } from '../utils-hoist/worldwide';
 interface CaptureConsoleOptions {
   levels?: string[];
 
-  // TODO(v9): Flip default value to `true` and adjust JSDoc!
   /**
-   * By default, Sentry will mark captured console messages as unhandled.
-   * Set this to `true` if you want to mark them as handled instead.
+   * By default, Sentry will mark captured console messages as handled.
+   * Set this to `false` if you want to mark them as unhandled instead.
    *
-   * Note: in v9 of the SDK, this option will default to `true`, meaning the default behavior will change to mark console messages as handled.
-   * @default false
+   * @default true
    */
   handled?: boolean;
 }
@@ -27,8 +25,7 @@ const INTEGRATION_NAME = 'CaptureConsole';
 
 const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
   const levels = options.levels || CONSOLE_LEVELS;
-  // TODO(v9): Flip default value to `true`
-  const handled = !!options.handled;
+  const handled = options.handled ?? true;
 
   return {
     name: INTEGRATION_NAME,
diff --git a/packages/core/test/lib/integrations/captureconsole.test.ts b/packages/core/test/lib/integrations/captureconsole.test.ts
@@ -306,8 +306,7 @@ describe('CaptureConsole setup', () => {
   });
 
   describe('exception mechanism', () => {
-    // TODO (v9): Flip this below after adjusting the default value for `handled` in the integration
-    it("marks captured exception's mechanism as unhandled by default", () => {
+    it("marks captured exception's mechanism as handled by default", () => {
       const captureConsole = captureConsoleIntegration({ levels: ['error'] });
       captureConsole.setup?.(mockClient);
 
@@ -326,7 +325,7 @@ describe('CaptureConsole setup', () => {
       expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
 
       expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
-        handled: false,
+        handled: true,
         type: 'console',
       });
     });
