Skip to content

Commit c6f4c95

Browse files
author
Luca Forstner
committed
ref(browser): Don't use Breadcrumbs integration in send event flow
1 parent 16356cb commit c6f4c95

File tree

3 files changed

+62
-37
lines changed

3 files changed

+62
-37
lines changed

packages/browser/src/client.ts

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1-
import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
1+
import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
22
import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
3-
import { createClientReportEnvelope, dsnToString, getGlobalObject, logger, serializeEnvelope } from '@sentry/utils';
3+
import {
4+
createClientReportEnvelope,
5+
dsnToString,
6+
getEventDescription,
7+
getGlobalObject,
8+
logger,
9+
serializeEnvelope,
10+
} from '@sentry/utils';
411

512
import { eventFromException, eventFromMessage } from './eventbuilder';
613
import { IS_DEBUG_BUILD } from './flags';
714
import { Breadcrumbs } from './integrations';
15+
import { BREADCRUMB_INTEGRATION_ID } from './integrations/breadcrumbs';
816
import { sendReport } from './transports/utils';
917

1018
const globalObject = getGlobalObject<Window>();
@@ -104,10 +112,34 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
104112
* @inheritDoc
105113
*/
106114
protected _sendEvent(event: Event): void {
107-
const integration = this.getIntegration(Breadcrumbs);
108-
if (integration) {
109-
integration.addSentryBreadcrumb(event);
115+
// We only want to add the sentry event breadcrumb when the user has the breadcrumb integration installed and
116+
// activated its`sentry` option.
117+
// We also do not want to use the `Breadcrumbs` class here directly, because we do not want it to be included in
118+
// bundles, if it is not used by the SDK.
119+
// 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
120+
// way for now.
121+
const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | null;
122+
if (
123+
breadcrumbIntegration &&
124+
// We check for definedness of `options`, even though it is not strictly necessary, because that access to
125+
// `.sentry` below does not throw, in case users provided their own integration with id "Breadcrumbs" that does
126+
// not have an`options` field
127+
breadcrumbIntegration.options &&
128+
breadcrumbIntegration.options.sentry
129+
) {
130+
getCurrentHub().addBreadcrumb(
131+
{
132+
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
133+
event_id: event.event_id,
134+
level: event.level,
135+
message: getEventDescription(event),
136+
},
137+
{
138+
event,
139+
},
140+
);
110141
}
142+
111143
super._sendEvent(event);
112144
}
113145

packages/browser/src/integrations/breadcrumbs.ts

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
22
/* eslint-disable max-lines */
33
import { getCurrentHub } from '@sentry/core';
4-
import { Event, Integration } from '@sentry/types';
4+
import { Integration } from '@sentry/types';
55
import {
66
addInstrumentationHandler,
7-
getEventDescription,
87
getGlobalObject,
98
htmlTreeAsString,
109
parseUrl,
@@ -22,6 +21,8 @@ interface BreadcrumbsOptions {
2221
xhr: boolean;
2322
}
2423

24+
export const BREADCRUMB_INTEGRATION_ID = 'Breadcrumbs';
25+
2526
/**
2627
* Default Breadcrumbs instrumentations
2728
* TODO: Deprecated - with v6, this will be renamed to `Instrument`
@@ -30,21 +31,24 @@ export class Breadcrumbs implements Integration {
3031
/**
3132
* @inheritDoc
3233
*/
33-
public static id: string = 'Breadcrumbs';
34+
public static id: string = BREADCRUMB_INTEGRATION_ID;
3435

3536
/**
3637
* @inheritDoc
3738
*/
3839
public name: string = Breadcrumbs.id;
3940

40-
/** JSDoc */
41-
private readonly _options: BreadcrumbsOptions;
41+
/**
42+
* Options of the breadcrumbs integration.
43+
*/
44+
// This field is public, because we use it in the browser client to check if the `sentry` option is enabled.
45+
public readonly options: Readonly<BreadcrumbsOptions>;
4246

4347
/**
4448
* @inheritDoc
4549
*/
4650
public constructor(options?: Partial<BreadcrumbsOptions>) {
47-
this._options = {
51+
this.options = {
4852
console: true,
4953
dom: true,
5054
fetch: true,
@@ -55,26 +59,6 @@ export class Breadcrumbs implements Integration {
5559
};
5660
}
5761

58-
/**
59-
* Create a breadcrumb of `sentry` from the events themselves
60-
*/
61-
public addSentryBreadcrumb(event: Event): void {
62-
if (!this._options.sentry) {
63-
return;
64-
}
65-
getCurrentHub().addBreadcrumb(
66-
{
67-
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
68-
event_id: event.event_id,
69-
level: event.level,
70-
message: getEventDescription(event),
71-
},
72-
{
73-
event,
74-
},
75-
);
76-
}
77-
7862
/**
7963
* Instrument browser built-ins w/ breadcrumb capturing
8064
* - Console API
@@ -84,19 +68,19 @@ export class Breadcrumbs implements Integration {
8468
* - History API
8569
*/
8670
public setupOnce(): void {
87-
if (this._options.console) {
71+
if (this.options.console) {
8872
addInstrumentationHandler('console', _consoleBreadcrumb);
8973
}
90-
if (this._options.dom) {
91-
addInstrumentationHandler('dom', _domBreadcrumb(this._options.dom));
74+
if (this.options.dom) {
75+
addInstrumentationHandler('dom', _domBreadcrumb(this.options.dom));
9276
}
93-
if (this._options.xhr) {
77+
if (this.options.xhr) {
9478
addInstrumentationHandler('xhr', _xhrBreadcrumb);
9579
}
96-
if (this._options.fetch) {
80+
if (this.options.fetch) {
9781
addInstrumentationHandler('fetch', _fetchBreadcrumb);
9882
}
99-
if (this._options.history) {
83+
if (this.options.history) {
10084
addInstrumentationHandler('history', _historyBreadcrumb);
10185
}
10286
}

packages/core/src/baseclient.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,15 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
258258
}
259259
}
260260

261+
/**
262+
* Gets an installed integration by its `id`.
263+
*
264+
* @returns The installed integration or `undefined` if no integration with that `id` was installed.
265+
*/
266+
public getIntegrationById(integrationId: string): Integration | undefined {
267+
return this._integrations[integrationId];
268+
}
269+
261270
/**
262271
* @inheritDoc
263272
*/

0 commit comments

Comments
 (0)