Merge remote-tracking branch 'upstream/7.x' into feat/attachments

Author: timfish

packages/browser/src/client.ts

@@ -1,10 +1,18 @@
-import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
-import { AttachmentWithData, ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
-import { createClientReportEnvelope, dsnToString, getGlobalObject, logger, serializeEnvelope } from '@sentry/utils';
+import { BaseClient, getCurrentHub, getEnvelopeEndpointWithUrlEncodedAuth, Scope, SDK_VERSION } from '@sentry/core';
+import { AttachmentWithData,ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
+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>();
@@ -95,20 +103,44 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
   /**
    * @inheritDoc
    */
-  protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
-    event.platform = event.platform || 'javascript';
-    return super._prepareEvent(event, scope, hint);
+  public sendEvent(event: Event, attachments?: AttachmentWithData[]): void {
+    // 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, attachments);
   }
 
   /**
    * @inheritDoc
    */
-  protected _sendEvent(event: Event, attachments: AttachmentWithData[]): void {
-    const integration = this.getIntegration(Breadcrumbs);
-    if (integration) {
-      integration.addSentryBreadcrumb(event);
-    }
-    super._sendEvent(event, attachments);
+  protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
+    event.platform = event.platform || 'javascript';
+    return super._prepareEvent(event, scope, hint);
   }
 
   /**

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

@@ -261,6 +261,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
    */
@@ -544,15 +553,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     }
   }
 
-  /**
-   * Sends the passed event
-   * @param event The Sentry event to send
-   */
-  // TODO(v7): refactor: get rid of method?
-  protected _sendEvent(event: Event, attachments?: AttachmentWithData[]): void {
-    this.sendEvent(event, attachments);
-  }
-
   /**
    * Processes the event and logs an error in case of rejection
    * @param event
@@ -630,7 +630,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
           this._updateSessionFromEvent(session, processedEvent);
         }
 
-        this._sendEvent(processedEvent, this._attachmentsFromScope(scope));
+        this.sendEvent(processedEvent, this._attachmentsFromScope(scope));
         return processedEvent;
       })
       .then(null, reason => {

packages/node/test/index.test.ts

@@ -1,6 +1,6 @@
 import { initAndBind, SDK_VERSION } from '@sentry/core';
 import { getMainCarrier } from '@sentry/hub';
-import { Attachment, Integration } from '@sentry/types';
+import { AttachmentWithData,Integration } from '@sentry/types';
 import * as domain from 'domain';
 
 import {
@@ -76,7 +76,7 @@ describe('SentryNode', () => {
   });
 
   describe('breadcrumbs', () => {
-    let s: jest.SpyInstance<void, [Event, Attachment[]?]>;
+    let s: jest.SpyInstance<void, [Event, AttachmentWithData[]?]>;
 
     beforeEach(() => {
       s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 }));
@@ -107,7 +107,7 @@ describe('SentryNode', () => {
   });
 
   describe('capture', () => {
-    let s: jest.SpyInstance<void, [Event, Attachment[]?]>;
+    let s: jest.SpyInstance<void, [Event, AttachmentWithData[]?]>;
 
     beforeEach(() => {
       s = jest.spyOn(NodeClient.prototype, 'sendEvent').mockImplementation(async () => Promise.resolve({ code: 200 }));

packages/utils/test/normalize.test.ts

@@ -5,7 +5,6 @@
 import * as isModule from '../src/is';
 import { normalize } from '../src/normalize';
 import * as stacktraceModule from '../src/stacktrace';
-import { testOnlyIfNodeVersionAtLeast } from './testutils';
 
 describe('normalize()', () => {
   describe('acts as a pass-through for simple-cases', () => {
@@ -48,7 +47,7 @@ describe('normalize()', () => {
       });
     });
 
-    testOnlyIfNodeVersionAtLeast(8)('extracts data from `Event` objects', () => {
+    describe('extracts data from `Event` objects', () => {
       const isElement = jest.spyOn(isModule, 'isElement').mockReturnValue(true);
       const getAttribute = () => undefined;