From 1c3f37b5a5d4c7dea87e208050ebc8ec3396f84b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 16 Dec 2024 16:52:02 +0100 Subject: [PATCH] ref(core)!: Mark exceptions from `captureConsoleIntegration` as `handled: true` by default (#14734) 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 https://github.com/getsentry/sentry-javascript/pull/14664. --- .../captureConsole-attachStackTrace/init.js | 11 ++ .../subject.js | 8 + .../captureConsole-attachStackTrace/test.ts | 176 ++++++++++++++++++ .../integrations/captureConsole/test.ts | 3 +- docs/migration/v8-to-v9.md | 11 ++ .../core/src/integrations/captureconsole.ts | 11 +- .../lib/integrations/captureconsole.test.ts | 5 +- 7 files changed, 213 insertions(+), 12 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/captureConsole-attachStackTrace/test.ts 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 new file mode 100644 index 000000000000..58f171d50df7 --- /dev/null +++ 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 new file mode 100644 index 000000000000..54f94f5ca4b3 --- /dev/null +++ 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 new file mode 100644 index 000000000000..4f1aafb87c7d --- /dev/null +++ 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(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 index 6f8cfc20f4aa..3dca4c6e979c 100644 --- 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 index bb0cfe487da0..40bb335d65b5 100644 --- 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 index c180dcbe99e7..3b027f985e66 100644 --- 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 index 4d480757fff1..a0555334f100 100644 --- 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', }); });