ref: Don't wrap xhr/fetch for sentry breadcrumbs (#2615)

* ref: Don't wrap xhr/fetch for sentry breadcrumbs

* fix: Breadcrumb tests

* chore: Changelog

* Apply suggestions from code review

Co-authored-by: Kamil Ogórek <[email protected]>

* Revert "Apply suggestions from code review"

This reverts commit e60fe99.

* fix: Client

* ref: Fix if

Co-authored-by: Kamil Ogórek <[email protected]>

Author: HazAT

CHANGELOG.md

@@ -16,6 +16,7 @@
 - [core] fix: sent_at for envelope headers to use same clock #2597
 - [apm] fix: Improve bundle size by moving span status to @sentry/apm #2589
 - [apm] feat: No longer discard transactions instead mark them deadline exceeded #2588
+- [browser] fix: Emit Sentry Request breadcrumbs from inside the client (#2615)
 
 ## 5.15.5
 

packages/browser/src/client.ts

@@ -3,6 +3,7 @@ import { DsnLike, Event, EventHint } from '@sentry/types';
 import { getGlobalObject, logger } from '@sentry/utils';
 
 import { BrowserBackend, BrowserOptions } from './backend';
+import { Breadcrumbs } from './integrations';
 import { SDK_NAME, SDK_VERSION } from './version';
 
 /**
@@ -69,6 +70,35 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
     return super._prepareEvent(event, scope, hint);
   }
 
+  /**
+   * @inheritDoc
+   */
+  public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
+    let eventId: string | undefined = hint && hint.event_id;
+    this._processing = true;
+
+    this._processEvent(event, hint, scope)
+      .then(finalEvent => {
+        // We need to check for finalEvent in case beforeSend returned null
+        // We do this here to not parse any requests if they are outgoing to Sentry
+        // We log breadcrumbs if the integration has them enabled
+        if (finalEvent) {
+          eventId = finalEvent.event_id;
+          const integration = this.getIntegration(Breadcrumbs);
+          if (integration) {
+            integration.addSentryBreadcrumb(finalEvent);
+          }
+        }
+        this._processing = false;
+      })
+      .then(null, reason => {
+        logger.error(reason);
+        this._processing = false;
+      });
+
+    return eventId;
+  }
+
   /**
    * Show a report dialog to the user to send feedback to a specific event.
    *

packages/browser/src/integrations/breadcrumbs.ts

@@ -1,17 +1,14 @@
-import { API, getCurrentHub } from '@sentry/core';
-import { Integration, Severity } from '@sentry/types';
+import { getCurrentHub } from '@sentry/core';
+import { Event, Integration, Severity } from '@sentry/types';
 import {
   addInstrumentationHandler,
   getEventDescription,
   getGlobalObject,
   htmlTreeAsString,
-  logger,
   parseUrl,
   safeJoin,
 } from '@sentry/utils';
 
-import { BrowserClient } from '../client';
-
 /**
  * @hidden
  */
@@ -67,6 +64,26 @@ 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,
+      },
+    );
+  }
+
   /**
    * Creates breadcrumbs from console API calls
    */
@@ -151,11 +168,6 @@ export class Breadcrumbs implements Integration {
 
       return;
     }
-
-    // We only capture issued sentry requests
-    if (this._options.sentry && handlerData.xhr.__sentry_own_request__) {
-      addSentryBreadcrumb(handlerData.args[0]);
-    }
   }
 
   /**
@@ -167,22 +179,9 @@ export class Breadcrumbs implements Integration {
       return;
     }
 
-    const client = getCurrentHub().getClient<BrowserClient>();
-    const dsn = client && client.getDsn();
-    if (this._options.sentry && dsn) {
-      const filterUrl = new API(dsn).getBaseApiEndpoint();
-      // if Sentry key appears in URL, don't capture it as a request
-      // but rather as our own 'sentry' type breadcrumb
-      if (
-        filterUrl &&
-        handlerData.fetchData.url.indexOf(filterUrl) !== -1 &&
-        handlerData.fetchData.method === 'POST' &&
-        handlerData.args[1] &&
-        handlerData.args[1].body
-      ) {
-        addSentryBreadcrumb(handlerData.args[1].body);
-        return;
-      }
+    if (handlerData.fetchData.url.match(/sentry_key/) && handlerData.fetchData.method === 'POST') {
+      // We will not create breadcrumbs for fetch requests that contain `sentry_key` (internal sentry requests)
+      return;
     }
 
     if (handlerData.error) {
@@ -306,26 +305,3 @@ export class Breadcrumbs implements Integration {
     }
   }
 }
-
-/**
- * Create a breadcrumb of `sentry` from the events themselves
- */
-function addSentryBreadcrumb(serializedData: string): void {
-  // There's always something that can go wrong with deserialization...
-  try {
-    const event = JSON.parse(serializedData);
-    getCurrentHub().addBreadcrumb(
-      {
-        category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
-        event_id: event.event_id,
-        level: event.level || Severity.fromString('error'),
-        message: getEventDescription(event),
-      },
-      {
-        event,
-      },
-    );
-  } catch (_oO) {
-    logger.error('Error while adding sentry type breadcrumb');
-  }
-}

packages/browser/test/integration/suites/breadcrumbs.js

@@ -84,115 +84,6 @@ describe("breadcrumbs", function() {
     }
   );
 
-  it(
-    optional(
-      "should transform XMLHttpRequests with events to the Sentry store endpoint as sentry.event type breadcrumb",
-      IS_LOADER
-    ),
-    function() {
-      return runInSandbox(sandbox, { manual: true }, function() {
-        var store =
-          document.location.protocol +
-          "//" +
-          document.location.hostname +
-          (document.location.port ? ":" + document.location.port : "") +
-          "/api/1/store/" +
-          "?sentry_key=1337";
-
-        var xhr = new XMLHttpRequest();
-        xhr.open("POST", store);
-        xhr.send('{"message":"someMessage","level":"warning"}');
-        waitForXHR(xhr, function() {
-          Sentry.captureMessage("test");
-          window.finalizeManualTest();
-        });
-      }).then(function(summary) {
-        // The async loader doesn't wrap XHR
-        if (IS_LOADER) {
-          return;
-        }
-        assert.equal(summary.breadcrumbs.length, 1);
-        assert.equal(summary.breadcrumbs[0].category, "sentry.event");
-        assert.equal(summary.breadcrumbs[0].level, "warning");
-        assert.equal(summary.breadcrumbs[0].message, "someMessage");
-      });
-    }
-  );
-
-  it(
-    optional(
-      "should transform XMLHttpRequests with transactions type to the Sentry store endpoint as sentry.transaction type breadcrumb",
-      IS_LOADER
-    ),
-    function() {
-      return runInSandbox(sandbox, { manual: true }, function() {
-        var store =
-          document.location.protocol +
-          "//" +
-          document.location.hostname +
-          (document.location.port ? ":" + document.location.port : "") +
-          "/api/1/store/" +
-          "?sentry_key=1337";
-
-        var xhr = new XMLHttpRequest();
-        xhr.open("POST", store);
-        xhr.send(
-          '{"message":"someMessage","transaction":"wat","level":"warning", "type": "transaction"}'
-        );
-        waitForXHR(xhr, function() {
-          Sentry.captureMessage("test");
-          window.finalizeManualTest();
-        });
-      }).then(function(summary) {
-        // The async loader doesn't wrap XHR
-        if (IS_LOADER) {
-          return;
-        }
-        assert.equal(summary.breadcrumbs.length, 1);
-        assert.equal(summary.breadcrumbs[0].category, "sentry.transaction");
-        assert.equal(summary.breadcrumbs[0].level, "warning");
-        assert.equal(summary.breadcrumbs[0].message, "someMessage");
-      });
-    }
-  );
-
-  it(
-    optional(
-      "should not transform XMLHttpRequests with transactions attribute to the Sentry store endpoint as sentry.transaction type breadcrumb",
-      IS_LOADER
-    ),
-    function() {
-      return runInSandbox(sandbox, { manual: true }, function() {
-        var store =
-          document.location.protocol +
-          "//" +
-          document.location.hostname +
-          (document.location.port ? ":" + document.location.port : "") +
-          "/api/1/store/" +
-          "?sentry_key=1337";
-
-        var xhr = new XMLHttpRequest();
-        xhr.open("POST", store);
-        xhr.send(
-          '{"message":"someMessage","transaction":"wat","level":"warning"}'
-        );
-        waitForXHR(xhr, function() {
-          Sentry.captureMessage("test");
-          window.finalizeManualTest();
-        });
-      }).then(function(summary) {
-        // The async loader doesn't wrap XHR
-        if (IS_LOADER) {
-          return;
-        }
-        assert.equal(summary.breadcrumbs.length, 1);
-        assert.equal(summary.breadcrumbs[0].category, "sentry.event");
-        assert.equal(summary.breadcrumbs[0].level, "warning");
-        assert.equal(summary.breadcrumbs[0].message, "someMessage");
-      });
-    }
-  );
-
   it("should record a fetch request", function() {
     return runInSandbox(sandbox, { manual: true }, function() {
       fetch("/base/subjects/example.json", {