From ea5563b581fb34dfd1968fcc8ee42154a8595c02 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 12 Nov 2022 11:37:41 +0100 Subject: [PATCH 1/3] Move logic inside integration --- packages/browser/src/client.ts | 29 ++++--------------- .../browser/src/integrations/breadcrumbs.ts | 22 +++++++++++++- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 996600c8127f..78a942d1d179 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,6 +1,6 @@ -import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; +import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core'; import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types'; -import { createClientReportEnvelope, dsnToString, getEventDescription, logger, serializeEnvelope } from '@sentry/utils'; +import { createClientReportEnvelope, dsnToString, logger, serializeEnvelope } from '@sentry/utils'; import { eventFromException, eventFromMessage } from './eventbuilder'; import { WINDOW } from './helpers'; @@ -101,27 +101,10 @@ export class BrowserClient extends BaseClient { // bundles, if it is not used by the SDK. // This all sadly is a bit ugly, but we currently don't have a "pre-send" hook on the integrations so we do it this // way for now. - const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | null; - if ( - breadcrumbIntegration && - // We check for definedness of `options`, even though it is not strictly necessary, because that access to - // `.sentry` below does not throw, in case users provided their own integration with id "Breadcrumbs" that does - // not have an`options` field - breadcrumbIntegration.options && - breadcrumbIntegration.options.sentry - ) { - getCurrentHub().addBreadcrumb( - { - category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`, - event_id: event.event_id, - level: event.level, - message: getEventDescription(event), - }, - { - event, - }, - ); - } + const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | undefined; + // We check for definedness of `addSentryBreadcrumb` in case users provided their own integration with id + // "Breadcrumbs" that does not have this function. + breadcrumbIntegration?.addSentryBreadcrumb?.(event); super.sendEvent(event, hint); } diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 8c852f156346..cb7e81e27871 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -1,9 +1,10 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ /* eslint-disable max-lines */ import { getCurrentHub } from '@sentry/core'; -import { Integration } from '@sentry/types'; +import { Event, Integration } from '@sentry/types'; import { addInstrumentationHandler, + getEventDescription, htmlTreeAsString, parseUrl, safeJoin, @@ -85,6 +86,25 @@ export class Breadcrumbs implements Integration { addInstrumentationHandler('history', _historyBreadcrumb); } } + + /** + * Adds a breadcrumb for Sentry events or transactions if this option is enabled. + */ + public addSentryBreadcrumb(event: Event): void { + if (this.options.sentry) { + getCurrentHub().addBreadcrumb( + { + category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`, + event_id: event.event_id, + level: event.level, + message: getEventDescription(event), + }, + { + event, + }, + ); + } + } } /** From 226e91e8d22822b3202c9eab4efb1e4f9fefe507 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Nov 2022 11:43:09 +0100 Subject: [PATCH 2/3] Add sentry breadcrumb test --- .../unit/integrations/breadcrumbs.test.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 packages/browser/test/unit/integrations/breadcrumbs.test.ts diff --git a/packages/browser/test/unit/integrations/breadcrumbs.test.ts b/packages/browser/test/unit/integrations/breadcrumbs.test.ts new file mode 100644 index 000000000000..de5def8440e5 --- /dev/null +++ b/packages/browser/test/unit/integrations/breadcrumbs.test.ts @@ -0,0 +1,34 @@ +import { BrowserClient, Breadcrumbs, Hub, flush } from '../../../src'; +import { getCurrentHub } from '@sentry/core'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; + +const hub = new Hub(); + +jest.mock('@sentry/core', () => { + const original = jest.requireActual('@sentry/core'); + return { + ...original, + getCurrentHub: () => hub, + }; +}); + +describe('Breadcrumbs', () => { + it('Should add sentry breadcrumb', async () => { + const addBreadcrumb = jest.fn(); + hub.addBreadcrumb = addBreadcrumb; + + const client = new BrowserClient({ + ...getDefaultBrowserClientOptions(), + dsn: 'https://username@domain/123', + integrations: [new Breadcrumbs()], + }); + + getCurrentHub().bindClient(client); + + client.captureMessage('test'); + await flush(2000); + + expect(addBreadcrumb.mock.calls[0][0].category).toEqual('sentry.event'); + expect(addBreadcrumb.mock.calls[0][0].message).toEqual('test'); + }); +}); From 4df43bd8171e73e1c761012e73423ffc7ebacc36 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 30 Nov 2022 12:23:04 +0100 Subject: [PATCH 3/3] Doh. Linting --- packages/browser/test/unit/integrations/breadcrumbs.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/browser/test/unit/integrations/breadcrumbs.test.ts b/packages/browser/test/unit/integrations/breadcrumbs.test.ts index de5def8440e5..f2deb2302c3c 100644 --- a/packages/browser/test/unit/integrations/breadcrumbs.test.ts +++ b/packages/browser/test/unit/integrations/breadcrumbs.test.ts @@ -1,5 +1,6 @@ -import { BrowserClient, Breadcrumbs, Hub, flush } from '../../../src'; import { getCurrentHub } from '@sentry/core'; + +import { Breadcrumbs, BrowserClient, flush, Hub } from '../../../src'; import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; const hub = new Hub();