From 6f04e52f6d89fad591e95f92ff69bc3df2047e02 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 13 Dec 2024 13:17:37 +0100 Subject: [PATCH 1/5] ref(core): Log debug message when capturing error events --- packages/core/src/baseclient.ts | 10 +++++ packages/core/test/lib/baseclient.test.ts | 46 +++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index c394a0d77a95..d77597b23ec4 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -713,6 +713,16 @@ export abstract class BaseClient implements Client { * @param scope */ protected _captureEvent(event: Event, hint: EventHint = {}, scope?: Scope): PromiseLike { + if (DEBUG_BUILD && isErrorEvent(event)) { + logger.log( + `Captured error event \`${ + (event.exception && event.exception.values && event.exception.values[0] && event.exception.values[0].value) || + event.message || + '' + }\``, + ); + } + return this._processEvent(event, hint, scope).then( finalEvent => { return finalEvent.event_id; diff --git a/packages/core/test/lib/baseclient.test.ts b/packages/core/test/lib/baseclient.test.ts index d66ef05881d0..0432235a17a5 100644 --- a/packages/core/test/lib/baseclient.test.ts +++ b/packages/core/test/lib/baseclient.test.ts @@ -366,6 +366,22 @@ describe('BaseClient', () => { // `captureException` should bail right away this second time around and not get as far as calling this again expect(clientEventFromException).toHaveBeenCalledTimes(1); }); + + test('captures logger message', () => { + const logSpy = jest.spyOn(loggerModule.logger, 'log').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + client.captureException(new Error('test error here')); + client.captureException({}); + + expect(logSpy).toHaveBeenCalledTimes(2); + expect(logSpy).toBeCalledWith('Captured error event `test error here`'); + expect(logSpy).toBeCalledWith('Captured error event ``'); + + logSpy.mockRestore(); + }); }); describe('captureMessage', () => { @@ -442,6 +458,20 @@ describe('BaseClient', () => { }), ); }); + + test('captures logger message', () => { + const logSpy = jest.spyOn(loggerModule.logger, 'log').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + client.captureMessage('test error here'); + + expect(logSpy).toHaveBeenCalledTimes(1); + expect(logSpy).toBeCalledWith('Captured error event `test error here`'); + + logSpy.mockRestore(); + }); }); describe('captureEvent() / prepareEvent()', () => { @@ -1658,6 +1688,22 @@ describe('BaseClient', () => { message: 'hello', }); }); + + test('captures logger message', () => { + const logSpy = jest.spyOn(loggerModule.logger, 'log').mockImplementation(() => undefined); + + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + + client.captureEvent({ message: 'hello' }); + // transactions are ignored and not logged + client.captureEvent({ type: 'transaction', message: 'hello 2' }); + + expect(logSpy).toHaveBeenCalledTimes(1); + expect(logSpy).toBeCalledWith('Captured error event `hello`'); + + logSpy.mockRestore(); + }); }); describe('integrations', () => { From da5f6303f5ed3251db58baece195ac92ad655ac3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 16 Dec 2024 08:59:43 +0100 Subject: [PATCH 2/5] fix size limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 6e73c9234c09..45324236f6ac 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -93,7 +93,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'feedbackIntegration'), gzip: true, - limit: '41 KB', + limit: '42 KB', }, { name: '@sentry/browser (incl. sendFeedback)', From d42ce46ca0aeefe63d9431ad0fea4188010ee39a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 16 Dec 2024 11:35:57 +0100 Subject: [PATCH 3/5] re-use `getPossibleEventMessages` util --- packages/core/src/baseclient.ts | 9 ++---- .../core/src/integrations/inboundfilters.ts | 28 ++--------------- packages/core/src/utils/eventUtils.ts | 31 +++++++++++++++++++ 3 files changed, 35 insertions(+), 33 deletions(-) create mode 100644 packages/core/src/utils/eventUtils.ts diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index d77597b23ec4..025631218c58 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -52,6 +52,7 @@ import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-h import { parseSampleRate } from './utils/parseSampleRate'; import { prepareEvent } from './utils/prepareEvent'; import { showSpanDropWarning } from './utils/spanUtils'; +import { getPossibleEventMessages } from './utils/eventUtils'; const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured."; @@ -714,13 +715,7 @@ export abstract class BaseClient implements Client { */ protected _captureEvent(event: Event, hint: EventHint = {}, scope?: Scope): PromiseLike { if (DEBUG_BUILD && isErrorEvent(event)) { - logger.log( - `Captured error event \`${ - (event.exception && event.exception.values && event.exception.values[0] && event.exception.values[0].value) || - event.message || - '' - }\``, - ); + logger.log(`Captured error event \`${getPossibleEventMessages(event)[0] || ''}\``); } return this._processEvent(event, hint, scope).then( diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 9d9f803a69f5..1fbbd211ebc1 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -5,6 +5,7 @@ import { defineIntegration } from '../integration'; import { logger } from '../utils-hoist/logger'; import { getEventDescription } from '../utils-hoist/misc'; import { stringMatchesSomePattern } from '../utils-hoist/string'; +import { getPossibleEventMessages } from '../utils/eventUtils'; // "Script error." is hard coded into browsers for errors that it can't read. // this is the result of a script being pulled in from an external domain and CORS. @@ -117,7 +118,7 @@ function _isIgnoredError(event: Event, ignoreErrors?: Array): b return false; } - return _getPossibleEventMessages(event).some(message => stringMatchesSomePattern(message, ignoreErrors)); + return getPossibleEventMessages(event).some(message => stringMatchesSomePattern(message, ignoreErrors)); } function _isIgnoredTransaction(event: Event, ignoreTransactions?: Array): boolean { @@ -147,32 +148,7 @@ function _isAllowedUrl(event: Event, allowUrls?: Array): boolea return !url ? true : stringMatchesSomePattern(url, allowUrls); } -function _getPossibleEventMessages(event: Event): string[] { - const possibleMessages: string[] = []; - if (event.message) { - possibleMessages.push(event.message); - } - - let lastException; - try { - // @ts-expect-error Try catching to save bundle size - lastException = event.exception.values[event.exception.values.length - 1]; - } catch (e) { - // try catching to save bundle size checking existence of variables - } - - if (lastException) { - if (lastException.value) { - possibleMessages.push(lastException.value); - if (lastException.type) { - possibleMessages.push(`${lastException.type}: ${lastException.value}`); - } - } - } - - return possibleMessages; -} function _isSentryError(event: Event): boolean { try { diff --git a/packages/core/src/utils/eventUtils.ts b/packages/core/src/utils/eventUtils.ts new file mode 100644 index 000000000000..c0af92441cd0 --- /dev/null +++ b/packages/core/src/utils/eventUtils.ts @@ -0,0 +1,31 @@ +import type { Event } from '../types-hoist'; + +/** + * Get a list of possible event messages from a Sentry event. + */ +export function getPossibleEventMessages(event: Event): string[] { + const possibleMessages: string[] = []; + + if (event.message) { + possibleMessages.push(event.message); + } + + let lastException; + try { + // @ts-expect-error Try catching to save bundle size + lastException = event.exception.values[event.exception.values.length - 1]; + } catch (e) { + // try catching to save bundle size checking existence of variables + } + + if (lastException) { + if (lastException.value) { + possibleMessages.push(lastException.value); + if (lastException.type) { + possibleMessages.push(`${lastException.type}: ${lastException.value}`); + } + } + } + + return possibleMessages; +} From 1dfeb1a7ec7a54ca45acbe9cc62e37a703629a99 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 16 Dec 2024 12:04:07 +0100 Subject: [PATCH 4/5] small streamline --- packages/core/src/utils/eventUtils.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/core/src/utils/eventUtils.ts b/packages/core/src/utils/eventUtils.ts index c0af92441cd0..3d1fa16eca58 100644 --- a/packages/core/src/utils/eventUtils.ts +++ b/packages/core/src/utils/eventUtils.ts @@ -10,21 +10,17 @@ export function getPossibleEventMessages(event: Event): string[] { possibleMessages.push(event.message); } - let lastException; try { // @ts-expect-error Try catching to save bundle size - lastException = event.exception.values[event.exception.values.length - 1]; - } catch (e) { - // try catching to save bundle size checking existence of variables - } - - if (lastException) { - if (lastException.value) { + const lastException = event.exception.values[event.exception.values.length - 1]; + if (lastException && lastException.value) { possibleMessages.push(lastException.value); if (lastException.type) { possibleMessages.push(`${lastException.type}: ${lastException.value}`); } } + } catch (e) { + // ignore errors here } return possibleMessages; From 6ebf424d8a8b2a17ad3601283b934be186095535 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 16 Dec 2024 12:04:39 +0100 Subject: [PATCH 5/5] fix linting --- packages/core/src/baseclient.ts | 2 +- packages/core/src/integrations/inboundfilters.ts | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 025631218c58..727fec079b55 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -49,10 +49,10 @@ import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from '. import { consoleSandbox, logger } from './utils-hoist/logger'; import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc'; import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise'; +import { getPossibleEventMessages } from './utils/eventUtils'; import { parseSampleRate } from './utils/parseSampleRate'; import { prepareEvent } from './utils/prepareEvent'; import { showSpanDropWarning } from './utils/spanUtils'; -import { getPossibleEventMessages } from './utils/eventUtils'; const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured."; diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 1fbbd211ebc1..223490bf9528 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -148,8 +148,6 @@ function _isAllowedUrl(event: Event, allowUrls?: Array): boolea return !url ? true : stringMatchesSomePattern(url, allowUrls); } - - function _isSentryError(event: Event): boolean { try { // @ts-expect-error can't be a sentry error if undefined