Skip to content

Commit c79bab6

Browse files
HazATkamilogorek
HazAT
and
kamilogorek
authored
ref: Don't wrap xhr/fetch for sentry breadcrumbs (getsentry#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 <kamil.ogorek@gmail.com> * Revert "Apply suggestions from code review" This reverts commit e60fe99. * fix: Client * ref: Fix if Co-authored-by: Kamil Ogórek <kamil.ogorek@gmail.com>
1 parent f1f0924 commit c79bab6

File tree

4 files changed

+56
-158
lines changed

4 files changed

+56
-158
lines changed

‎CHANGELOG.md

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

2021
## 5.15.5
2122

‎packages/browser/src/client.ts

+30
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { DsnLike, Event, EventHint } from '@sentry/types';
33
import { getGlobalObject, logger } from '@sentry/utils';
44

55
import { BrowserBackend, BrowserOptions } from './backend';
6+
import { Breadcrumbs } from './integrations';
67
import { SDK_NAME, SDK_VERSION } from './version';
78

89
/**
@@ -69,6 +70,35 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
6970
return super._prepareEvent(event, scope, hint);
7071
}
7172

73+
/**
74+
* @inheritDoc
75+
*/
76+
public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
77+
let eventId: string | undefined = hint && hint.event_id;
78+
this._processing = true;
79+
80+
this._processEvent(event, hint, scope)
81+
.then(finalEvent => {
82+
// We need to check for finalEvent in case beforeSend returned null
83+
// We do this here to not parse any requests if they are outgoing to Sentry
84+
// We log breadcrumbs if the integration has them enabled
85+
if (finalEvent) {
86+
eventId = finalEvent.event_id;
87+
const integration = this.getIntegration(Breadcrumbs);
88+
if (integration) {
89+
integration.addSentryBreadcrumb(finalEvent);
90+
}
91+
}
92+
this._processing = false;
93+
})
94+
.then(null, reason => {
95+
logger.error(reason);
96+
this._processing = false;
97+
});
98+
99+
return eventId;
100+
}
101+
72102
/**
73103
* Show a report dialog to the user to send feedback to a specific event.
74104
*

‎packages/browser/src/integrations/breadcrumbs.ts

+25-49
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
1-
import { API, getCurrentHub } from '@sentry/core';
2-
import { Integration, Severity } from '@sentry/types';
1+
import { getCurrentHub } from '@sentry/core';
2+
import { Event, Integration, Severity } from '@sentry/types';
33
import {
44
addInstrumentationHandler,
55
getEventDescription,
66
getGlobalObject,
77
htmlTreeAsString,
8-
logger,
98
parseUrl,
109
safeJoin,
1110
} from '@sentry/utils';
1211

13-
import { BrowserClient } from '../client';
14-
1512
/**
1613
* @hidden
1714
*/
@@ -67,6 +64,26 @@ export class Breadcrumbs implements Integration {
6764
};
6865
}
6966

67+
/**
68+
* Create a breadcrumb of `sentry` from the events themselves
69+
*/
70+
public addSentryBreadcrumb(event: Event): void {
71+
if (!this._options.sentry) {
72+
return;
73+
}
74+
getCurrentHub().addBreadcrumb(
75+
{
76+
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
77+
event_id: event.event_id,
78+
level: event.level,
79+
message: getEventDescription(event),
80+
},
81+
{
82+
event,
83+
},
84+
);
85+
}
86+
7087
/**
7188
* Creates breadcrumbs from console API calls
7289
*/
@@ -151,11 +168,6 @@ export class Breadcrumbs implements Integration {
151168

152169
return;
153170
}
154-
155-
// We only capture issued sentry requests
156-
if (this._options.sentry && handlerData.xhr.__sentry_own_request__) {
157-
addSentryBreadcrumb(handlerData.args[0]);
158-
}
159171
}
160172

161173
/**
@@ -167,22 +179,9 @@ export class Breadcrumbs implements Integration {
167179
return;
168180
}
169181

170-
const client = getCurrentHub().getClient<BrowserClient>();
171-
const dsn = client && client.getDsn();
172-
if (this._options.sentry && dsn) {
173-
const filterUrl = new API(dsn).getBaseApiEndpoint();
174-
// if Sentry key appears in URL, don't capture it as a request
175-
// but rather as our own 'sentry' type breadcrumb
176-
if (
177-
filterUrl &&
178-
handlerData.fetchData.url.indexOf(filterUrl) !== -1 &&
179-
handlerData.fetchData.method === 'POST' &&
180-
handlerData.args[1] &&
181-
handlerData.args[1].body
182-
) {
183-
addSentryBreadcrumb(handlerData.args[1].body);
184-
return;
185-
}
182+
if (handlerData.fetchData.url.match(/sentry_key/) && handlerData.fetchData.method === 'POST') {
183+
// We will not create breadcrumbs for fetch requests that contain `sentry_key` (internal sentry requests)
184+
return;
186185
}
187186

188187
if (handlerData.error) {
@@ -306,26 +305,3 @@ export class Breadcrumbs implements Integration {
306305
}
307306
}
308307
}
309-
310-
/**
311-
* Create a breadcrumb of `sentry` from the events themselves
312-
*/
313-
function addSentryBreadcrumb(serializedData: string): void {
314-
// There's always something that can go wrong with deserialization...
315-
try {
316-
const event = JSON.parse(serializedData);
317-
getCurrentHub().addBreadcrumb(
318-
{
319-
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
320-
event_id: event.event_id,
321-
level: event.level || Severity.fromString('error'),
322-
message: getEventDescription(event),
323-
},
324-
{
325-
event,
326-
},
327-
);
328-
} catch (_oO) {
329-
logger.error('Error while adding sentry type breadcrumb');
330-
}
331-
}

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

-109
Original file line numberDiff line numberDiff line change
@@ -84,115 +84,6 @@ describe("breadcrumbs", function() {
8484
}
8585
);
8686

87-
it(
88-
optional(
89-
"should transform XMLHttpRequests with events to the Sentry store endpoint as sentry.event type breadcrumb",
90-
IS_LOADER
91-
),
92-
function() {
93-
return runInSandbox(sandbox, { manual: true }, function() {
94-
var store =
95-
document.location.protocol +
96-
"//" +
97-
document.location.hostname +
98-
(document.location.port ? ":" + document.location.port : "") +
99-
"/api/1/store/" +
100-
"?sentry_key=1337";
101-
102-
var xhr = new XMLHttpRequest();
103-
xhr.open("POST", store);
104-
xhr.send('{"message":"someMessage","level":"warning"}');
105-
waitForXHR(xhr, function() {
106-
Sentry.captureMessage("test");
107-
window.finalizeManualTest();
108-
});
109-
}).then(function(summary) {
110-
// The async loader doesn't wrap XHR
111-
if (IS_LOADER) {
112-
return;
113-
}
114-
assert.equal(summary.breadcrumbs.length, 1);
115-
assert.equal(summary.breadcrumbs[0].category, "sentry.event");
116-
assert.equal(summary.breadcrumbs[0].level, "warning");
117-
assert.equal(summary.breadcrumbs[0].message, "someMessage");
118-
});
119-
}
120-
);
121-
122-
it(
123-
optional(
124-
"should transform XMLHttpRequests with transactions type to the Sentry store endpoint as sentry.transaction type breadcrumb",
125-
IS_LOADER
126-
),
127-
function() {
128-
return runInSandbox(sandbox, { manual: true }, function() {
129-
var store =
130-
document.location.protocol +
131-
"//" +
132-
document.location.hostname +
133-
(document.location.port ? ":" + document.location.port : "") +
134-
"/api/1/store/" +
135-
"?sentry_key=1337";
136-
137-
var xhr = new XMLHttpRequest();
138-
xhr.open("POST", store);
139-
xhr.send(
140-
'{"message":"someMessage","transaction":"wat","level":"warning", "type": "transaction"}'
141-
);
142-
waitForXHR(xhr, function() {
143-
Sentry.captureMessage("test");
144-
window.finalizeManualTest();
145-
});
146-
}).then(function(summary) {
147-
// The async loader doesn't wrap XHR
148-
if (IS_LOADER) {
149-
return;
150-
}
151-
assert.equal(summary.breadcrumbs.length, 1);
152-
assert.equal(summary.breadcrumbs[0].category, "sentry.transaction");
153-
assert.equal(summary.breadcrumbs[0].level, "warning");
154-
assert.equal(summary.breadcrumbs[0].message, "someMessage");
155-
});
156-
}
157-
);
158-
159-
it(
160-
optional(
161-
"should not transform XMLHttpRequests with transactions attribute to the Sentry store endpoint as sentry.transaction type breadcrumb",
162-
IS_LOADER
163-
),
164-
function() {
165-
return runInSandbox(sandbox, { manual: true }, function() {
166-
var store =
167-
document.location.protocol +
168-
"//" +
169-
document.location.hostname +
170-
(document.location.port ? ":" + document.location.port : "") +
171-
"/api/1/store/" +
172-
"?sentry_key=1337";
173-
174-
var xhr = new XMLHttpRequest();
175-
xhr.open("POST", store);
176-
xhr.send(
177-
'{"message":"someMessage","transaction":"wat","level":"warning"}'
178-
);
179-
waitForXHR(xhr, function() {
180-
Sentry.captureMessage("test");
181-
window.finalizeManualTest();
182-
});
183-
}).then(function(summary) {
184-
// The async loader doesn't wrap XHR
185-
if (IS_LOADER) {
186-
return;
187-
}
188-
assert.equal(summary.breadcrumbs.length, 1);
189-
assert.equal(summary.breadcrumbs[0].category, "sentry.event");
190-
assert.equal(summary.breadcrumbs[0].level, "warning");
191-
assert.equal(summary.breadcrumbs[0].message, "someMessage");
192-
});
193-
}
194-
);
195-
19687
it("should record a fetch request", function() {
19788
return runInSandbox(sandbox, { manual: true }, function() {
19889
fetch("/base/subjects/example.json", {

0 commit comments

Comments
 (0)