Skip to content

Commit 0335cf7

Browse files
authored
fix(browser): Respect manually set sentry tracing headers in XHR requests (#16184)
Analogously to #16183 for `fetch`, this patch ensures that manually set `sentry-trace` and `baggage` headers in XHR requests are respected by our XHR instrumentation and not overwritten. This patch also fixes a bug where we'd append multiple `sentry-trace` or `baggage` values in case the header was somehow set more than once (analogously to #13907 for `fetch`)
1 parent 69224b8 commit 0335cf7

File tree

5 files changed

+89
-5
lines changed

5 files changed

+89
-5
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
const xhr = new XMLHttpRequest();
2+
3+
xhr.open('GET', 'http://sentry-test-site.example/1');
4+
xhr.setRequestHeader('X-Test-Header', 'existing-header');
5+
xhr.setRequestHeader('baggage', 'someVendor-foo=bar');
6+
7+
xhr.send();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect } from '@playwright/test';
2+
import { TRACEPARENT_REGEXP } from '@sentry/core';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { shouldSkipTracingTest } from '../../../../utils/helpers';
5+
6+
sentryTest('merges `baggage` headers of pre-existing non-sentry XHR requests', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipTracingTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const url = await getLocalTestUrl({ testDir: __dirname });
12+
13+
const requestPromise = page.waitForRequest('http://sentry-test-site.example/1');
14+
15+
await page.goto(url);
16+
17+
const request = await requestPromise;
18+
19+
const requestHeaders = request.headers();
20+
expect(requestHeaders).toMatchObject({
21+
'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP),
22+
baggage: expect.stringMatching(/^someVendor-foo=bar, sentry-.*$/),
23+
'x-test-header': 'existing-header',
24+
});
25+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const xhr = new XMLHttpRequest();
2+
3+
xhr.open('GET', 'http://sentry-test-site.example/1');
4+
xhr.setRequestHeader('X-Test-Header', 'existing-header');
5+
xhr.setRequestHeader('sentry-trace', '123-abc-1');
6+
xhr.setRequestHeader('baggage', ' sentry-release=1.1.1, sentry-trace_id=123');
7+
8+
xhr.send();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../../utils/fixtures';
3+
import { shouldSkipTracingTest } from '../../../../utils/helpers';
4+
5+
sentryTest(
6+
'attaches manually passed in `sentry-trace` and `baggage` headers to XHR requests',
7+
async ({ getLocalTestUrl, page }) => {
8+
if (shouldSkipTracingTest()) {
9+
sentryTest.skip();
10+
}
11+
12+
const url = await getLocalTestUrl({ testDir: __dirname });
13+
14+
const requestPromise = page.waitForRequest('http://sentry-test-site.example/1');
15+
16+
await page.goto(url);
17+
18+
const request = await requestPromise;
19+
20+
const requestHeaders = request.headers();
21+
expect(requestHeaders).toMatchObject({
22+
'sentry-trace': '123-abc-1',
23+
baggage: 'sentry-release=1.1.1, sentry-trace_id=123',
24+
'x-test-header': 'existing-header',
25+
});
26+
},
27+
);

packages/browser/src/tracing/request.ts

+22-5
Original file line numberDiff line numberDiff line change
@@ -420,21 +420,38 @@ function setHeaderOnXhr(
420420
sentryTraceHeader: string,
421421
sentryBaggageHeader: string | undefined,
422422
): void {
423+
const originalHeaders = xhr.__sentry_xhr_v3__?.request_headers;
424+
425+
if (originalHeaders?.['sentry-trace']) {
426+
// bail if a sentry-trace header is already set
427+
return;
428+
}
429+
423430
try {
424431
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
425432
xhr.setRequestHeader!('sentry-trace', sentryTraceHeader);
426433
if (sentryBaggageHeader) {
427-
// From MDN: "If this method is called several times with the same header, the values are merged into one single request header."
428-
// We can therefore simply set a baggage header without checking what was there before
429-
// https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
430-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
431-
xhr.setRequestHeader!('baggage', sentryBaggageHeader);
434+
// only add our headers if
435+
// - no pre-existing baggage header exists
436+
// - or it is set and doesn't yet contain sentry values
437+
const originalBaggageHeader = originalHeaders?.['baggage'];
438+
if (!originalBaggageHeader || !baggageHeaderHasSentryValues(originalBaggageHeader)) {
439+
// From MDN: "If this method is called several times with the same header, the values are merged into one single request header."
440+
// We can therefore simply set a baggage header without checking what was there before
441+
// https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader
442+
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
443+
xhr.setRequestHeader!('baggage', sentryBaggageHeader);
444+
}
432445
}
433446
} catch (_) {
434447
// Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
435448
}
436449
}
437450

451+
function baggageHeaderHasSentryValues(baggageHeader: string): boolean {
452+
return baggageHeader.split(',').some(value => value.trim().startsWith('sentry-'));
453+
}
454+
438455
function getFullURL(url: string): string | undefined {
439456
try {
440457
// By adding a base URL to new URL(), this will also work for relative urls

0 commit comments

Comments
 (0)