ref(browser): Don't use Breadcrumbs integration in send event flow (#5024)

Author: Luca Forstner

packages/browser/src/client.ts

@@ -1,10 +1,18 @@
-import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
+import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
 import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
-import { createClientReportEnvelope, dsnToString, getGlobalObject, logger, serializeEnvelope } from '@sentry/utils';
+import {
+  createClientReportEnvelope,
+  dsnToString,
+  getEventDescription,
+  getGlobalObject,
+  logger,
+  serializeEnvelope,
+} from '@sentry/utils';
 
 import { eventFromException, eventFromMessage } from './eventbuilder';
 import { IS_DEBUG_BUILD } from './flags';
 import { Breadcrumbs } from './integrations';
+import { BREADCRUMB_INTEGRATION_ID } from './integrations/breadcrumbs';
 import { sendReport } from './transports/utils';
 
 const globalObject = getGlobalObject<Window>();
@@ -104,10 +112,34 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
    * @inheritDoc
    */
   protected _sendEvent(event: Event): void {
-    const integration = this.getIntegration(Breadcrumbs);
-    if (integration) {
-      integration.addSentryBreadcrumb(event);
+    // We only want to add the sentry event breadcrumb when the user has the breadcrumb integration installed and
+    // activated its `sentry` option.
+    // We also do not want to use the `Breadcrumbs` class here directly, because we do not want it to be included in
+    // 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,
+        },
+      );
     }
+
     super._sendEvent(event);
   }
 

packages/browser/src/integrations/breadcrumbs.ts

@@ -1,10 +1,9 @@
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
 /* eslint-disable max-lines */
 import { getCurrentHub } from '@sentry/core';
-import { Event, Integration } from '@sentry/types';
+import { Integration } from '@sentry/types';
 import {
   addInstrumentationHandler,
-  getEventDescription,
   getGlobalObject,
   htmlTreeAsString,
   parseUrl,
@@ -22,6 +21,8 @@ interface BreadcrumbsOptions {
   xhr: boolean;
 }
 
+export const BREADCRUMB_INTEGRATION_ID = 'Breadcrumbs';
+
 /**
  * Default Breadcrumbs instrumentations
  * TODO: Deprecated - with v6, this will be renamed to `Instrument`
@@ -30,21 +31,24 @@ export class Breadcrumbs implements Integration {
   /**
    * @inheritDoc
    */
-  public static id: string = 'Breadcrumbs';
+  public static id: string = BREADCRUMB_INTEGRATION_ID;
 
   /**
    * @inheritDoc
    */
   public name: string = Breadcrumbs.id;
 
-  /** JSDoc */
-  private readonly _options: BreadcrumbsOptions;
+  /**
+   * Options of the breadcrumbs integration.
+   */
+  // This field is public, because we use it in the browser client to check if the `sentry` option is enabled.
+  public readonly options: Readonly<BreadcrumbsOptions>;
 
   /**
    * @inheritDoc
    */
   public constructor(options?: Partial<BreadcrumbsOptions>) {
-    this._options = {
+    this.options = {
       console: true,
       dom: true,
       fetch: true,
@@ -55,26 +59,6 @@ export class Breadcrumbs implements Integration {
     };
   }
 
-  /**
-   * Create a breadcrumb of `sentry` from the events themselves
-   */
-  public addSentryBreadcrumb(event: Event): void {
-    if (!this._options.sentry) {
-      return;
-    }
-    getCurrentHub().addBreadcrumb(
-      {
-        category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
-        event_id: event.event_id,
-        level: event.level,
-        message: getEventDescription(event),
-      },
-      {
-        event,
-      },
-    );
-  }
-
   /**
    * Instrument browser built-ins w/ breadcrumb capturing
    *  - Console API
@@ -84,19 +68,19 @@ export class Breadcrumbs implements Integration {
    *  - History API
    */
   public setupOnce(): void {
-    if (this._options.console) {
+    if (this.options.console) {
       addInstrumentationHandler('console', _consoleBreadcrumb);
     }
-    if (this._options.dom) {
-      addInstrumentationHandler('dom', _domBreadcrumb(this._options.dom));
+    if (this.options.dom) {
+      addInstrumentationHandler('dom', _domBreadcrumb(this.options.dom));
     }
-    if (this._options.xhr) {
+    if (this.options.xhr) {
       addInstrumentationHandler('xhr', _xhrBreadcrumb);
     }
-    if (this._options.fetch) {
+    if (this.options.fetch) {
       addInstrumentationHandler('fetch', _fetchBreadcrumb);
     }
-    if (this._options.history) {
+    if (this.options.history) {
       addInstrumentationHandler('history', _historyBreadcrumb);
     }
   }

packages/core/src/baseclient.ts

@@ -258,6 +258,15 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     }
   }
 
+  /**
+   * Gets an installed integration by its `id`.
+   *
+   * @returns The installed integration or `undefined` if no integration with that `id` was installed.
+   */
+  public getIntegrationById(integrationId: string): Integration | undefined {
+    return this._integrations[integrationId];
+  }
+
   /**
    * @inheritDoc
    */