Skip to content

Commit

Permalink
feat(core): Add handled option to captureConsoleIntegration (#14664)
Browse files Browse the repository at this point in the history 
Add a `handled` option to the `captureConsoleIntegration`.
Right now, for v8, it will default to `false` as previously but it gives
users the option to override the handled value. In v9, we will flip the
default value to `true` after discussing this offline with the team. In
this integration we can never be sure about the `handled` state at all.
But flipping it to handled by default seems reasonable.

Co-authored-by: Luca Forstner <luca.forstner@sentry.io>
  • Loading branch information
@Lms24 @lforst
Lms24 and lforst authored Dec 12, 2024
1 parent 0aa7d95 commit c49b247
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 21 deletions.
10 changes: 10 additions & 0 deletions dev-packages/browser-integration-tests/suites/integrations/captureConsole/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
extra: {
arguments: ['console log'],
},
message: 'console log',
}),
);
expect(logEvent?.exception).toBeUndefined();
Expand All @@ -40,6 +41,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
extra: {
arguments: ['console warn'],
},
message: 'console warn',
}),
);
expect(warnEvent?.exception).toBeUndefined();
Expand All @@ -50,6 +52,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
extra: {
arguments: ['console info'],
},
message: 'console info',
}),
);
expect(infoEvent?.exception).toBeUndefined();
Expand All @@ -60,6 +63,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
extra: {
arguments: ['console error'],
},
message: 'console error',
}),
);
expect(errorEvent?.exception).toBeUndefined();
Expand All @@ -70,6 +74,7 @@ sentryTest('it captures console messages correctly', async ({ getLocalTestUrl, p
extra: {
arguments: ['console trace'],
},
message: 'console trace',
}),
);
expect(traceEvent?.exception).toBeUndefined();
Expand All @@ -90,6 +95,11 @@ 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,
type: 'console',
});
expect(traceWithErrorEvent).toEqual(
expect.objectContaining({
level: 'log',
Expand Down
18 changes: 15 additions & 3 deletions packages/core/src/integrations/captureconsole.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,24 @@ 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.
*
* 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
*/
handled?: boolean;
}

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;

return {
name: INTEGRATION_NAME,
Expand All @@ -30,7 +42,7 @@ const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
return;
}

consoleHandler(args, level);
consoleHandler(args, level, handled);
});
},
};
Expand All @@ -41,7 +53,7 @@ const _captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
*/
export const captureConsoleIntegration = defineIntegration(_captureConsoleIntegration);

function consoleHandler(args: unknown[], level: string): void {
function consoleHandler(args: unknown[], level: string, handled: boolean): void {
const captureContext: CaptureContext = {
level: severityLevelFromString(level),
extra: {
Expand All @@ -54,7 +66,7 @@ function consoleHandler(args: unknown[], level: string): void {
event.logger = 'console';

addExceptionMechanism(event, {
handled: false,
handled,
type: 'console',
});

Expand Down
85 changes: 67 additions & 18 deletions packages/core/test/lib/integrations/captureconsole.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,29 +305,78 @@ describe('CaptureConsole setup', () => {
}).not.toThrow();
});

it("marks captured exception's mechanism as unhandled", () => {
// const addExceptionMechanismSpy = jest.spyOn(utils, 'addExceptionMechanism');
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", () => {
const captureConsole = captureConsoleIntegration({ levels: ['error'] });
captureConsole.setup?.(mockClient);

const captureConsole = captureConsoleIntegration({ levels: ['error'] });
captureConsole.setup?.(mockClient);
const someError = new Error('some error');
GLOBAL_OBJ.console.error(someError);

const someError = new Error('some error');
GLOBAL_OBJ.console.error(someError);
const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
const someEvent: Event = {
exception: {
values: [{}],
},
};
addedEventProcessor(someEvent);

const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
const someEvent: Event = {
exception: {
values: [{}],
},
};
addedEventProcessor(someEvent);
expect(captureException).toHaveBeenCalledTimes(1);
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);

expect(captureException).toHaveBeenCalledTimes(1);
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);
expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'console',
});
});

it("marks captured exception's mechanism as handled if set in the options", () => {
const captureConsole = captureConsoleIntegration({ levels: ['error'], handled: true });
captureConsole.setup?.(mockClient);

expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'console',
const someError = new Error('some error');
GLOBAL_OBJ.console.error(someError);

const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
const someEvent: Event = {
exception: {
values: [{}],
},
};
addedEventProcessor(someEvent);

expect(captureException).toHaveBeenCalledTimes(1);
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);

expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: true,
type: 'console',
});
});

it("marks captured exception's mechanism as unhandled if set in the options", () => {
const captureConsole = captureConsoleIntegration({ levels: ['error'], handled: false });
captureConsole.setup?.(mockClient);

const someError = new Error('some error');
GLOBAL_OBJ.console.error(someError);

const addedEventProcessor = (mockScope.addEventProcessor as jest.Mock).mock.calls[0][0];
const someEvent: Event = {
exception: {
values: [{}],
},
};
addedEventProcessor(someEvent);

expect(captureException).toHaveBeenCalledTimes(1);
expect(mockScope.addEventProcessor).toHaveBeenCalledTimes(1);

expect(someEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
type: 'console',
});
});
});
});

0 comments on commit c49b247

Please sign in to comment.