From 0b84168dbbe71cf8903ef1dc9ca246c27843e689 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 May 2025 15:31:52 +0200 Subject: [PATCH 1/2] fix(browser): Respect manually set sentry tracing headers in XHR requests --- .../xhr-merged-baggage-headers/subject.js | 7 +++++ .../xhr-merged-baggage-headers/test.ts | 25 +++++++++++++++++ .../xhr-with-custom-sentry-headers/subject.js | 8 ++++++ .../xhr-with-custom-sentry-headers/test.ts | 27 +++++++++++++++++++ packages/browser/src/tracing/request.ts | 25 +++++++++++++---- 5 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/subject.js new file mode 100644 index 000000000000..584c264d1f0a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/subject.js @@ -0,0 +1,7 @@ +const xhr = new XMLHttpRequest(); + +xhr.open('GET', 'http://sentry-test-site.example/1'); +xhr.setRequestHeader('X-Test-Header', 'existing-header'); +xhr.setRequestHeader('baggage', 'someVendor-foo=bar'); + +xhr.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/test.ts new file mode 100644 index 000000000000..3086347cbd0c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-merged-baggage-headers/test.ts @@ -0,0 +1,25 @@ +import { expect } from '@playwright/test'; +import { TRACEPARENT_REGEXP } from '@sentry/core'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest('merges `baggage` headers of pre-existing non-sentry XHR requests', async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const requestPromise = page.waitForRequest('http://sentry-test-site.example/1'); + + await page.goto(url); + + const request = await requestPromise; + + const requestHeaders = request.headers(); + expect(requestHeaders).toMatchObject({ + 'sentry-trace': expect.stringMatching(TRACEPARENT_REGEXP), + baggage: expect.stringMatching(/^someVendor-foo=bar, sentry-.*$/), + 'x-test-header': 'existing-header', + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js new file mode 100644 index 000000000000..4e96d0a9be81 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js @@ -0,0 +1,8 @@ +const xhr = new XMLHttpRequest(); + +xhr.open('GET', 'http://sentry-test-site.example/1'); +xhr.setRequestHeader('X-Test-Header', 'existing-header'); +xhr.setRequestHeader('sentry-trace', '123-abc-1'); +xhr.setRequestHeader('baggage', 'sentry-release=1.1.1'); + +xhr.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts new file mode 100644 index 000000000000..6338703555a6 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts @@ -0,0 +1,27 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest( + 'attaches manually passed in `sentry-trace` and `baggage` headers to XHR requests', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + + const requestPromise = page.waitForRequest('http://sentry-test-site.example/1'); + + await page.goto(url); + + const request = await requestPromise; + + const requestHeaders = request.headers(); + expect(requestHeaders).toMatchObject({ + 'sentry-trace': '123-abc-1', + baggage: 'sentry-release=1.1.1', + 'x-test-header': 'existing-header', + }); + }, +); diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index 4fb816d1319d..a773107a8718 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -420,21 +420,36 @@ function setHeaderOnXhr( sentryTraceHeader: string, sentryBaggageHeader: string | undefined, ): void { + const originalHeaders = xhr.__sentry_xhr_v3__?.request_headers; + + if (originalHeaders?.['sentry-trace']) { + // bail if a sentry-trace header is already set + return; + } + try { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion xhr.setRequestHeader!('sentry-trace', sentryTraceHeader); if (sentryBaggageHeader) { - // From MDN: "If this method is called several times with the same header, the values are merged into one single request header." - // We can therefore simply set a baggage header without checking what was there before - // https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - xhr.setRequestHeader!('baggage', sentryBaggageHeader); + // bail if a pre-existing baggage header is set and already contains sentry values + const originalBaggageHeader = originalHeaders?.['baggage']; + if (!originalBaggageHeader || !baggageHeaderHasSentryValues(originalBaggageHeader)) { + // From MDN: "If this method is called several times with the same header, the values are merged into one single request header." + // We can therefore simply set a baggage header without checking what was there before + // https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/setRequestHeader + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + xhr.setRequestHeader!('baggage', sentryBaggageHeader); + } } } catch (_) { // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. } } +function baggageHeaderHasSentryValues(baggageHeader: string): boolean { + return baggageHeader.split(',').some(value => value.startsWith('sentry-')); +} + function getFullURL(url: string): string | undefined { try { // By adding a base URL to new URL(), this will also work for relative urls From 205b4f58dee76afe8f49b67b692aaad25a634187 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 5 May 2025 14:09:48 +0200 Subject: [PATCH 2/2] account for spaces in baggage header (+adjust test) --- .../request/xhr-with-custom-sentry-headers/subject.js | 2 +- .../tracing/request/xhr-with-custom-sentry-headers/test.ts | 2 +- packages/browser/src/tracing/request.ts | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js index 4e96d0a9be81..595ab4b67bac 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/subject.js @@ -3,6 +3,6 @@ const xhr = new XMLHttpRequest(); xhr.open('GET', 'http://sentry-test-site.example/1'); xhr.setRequestHeader('X-Test-Header', 'existing-header'); xhr.setRequestHeader('sentry-trace', '123-abc-1'); -xhr.setRequestHeader('baggage', 'sentry-release=1.1.1'); +xhr.setRequestHeader('baggage', ' sentry-release=1.1.1, sentry-trace_id=123'); xhr.send(); diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts index 6338703555a6..49d4b3091258 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/xhr-with-custom-sentry-headers/test.ts @@ -20,7 +20,7 @@ sentryTest( const requestHeaders = request.headers(); expect(requestHeaders).toMatchObject({ 'sentry-trace': '123-abc-1', - baggage: 'sentry-release=1.1.1', + baggage: 'sentry-release=1.1.1, sentry-trace_id=123', 'x-test-header': 'existing-header', }); }, diff --git a/packages/browser/src/tracing/request.ts b/packages/browser/src/tracing/request.ts index a773107a8718..cc987fca2966 100644 --- a/packages/browser/src/tracing/request.ts +++ b/packages/browser/src/tracing/request.ts @@ -431,7 +431,9 @@ function setHeaderOnXhr( // eslint-disable-next-line @typescript-eslint/no-non-null-assertion xhr.setRequestHeader!('sentry-trace', sentryTraceHeader); if (sentryBaggageHeader) { - // bail if a pre-existing baggage header is set and already contains sentry values + // only add our headers if + // - no pre-existing baggage header exists + // - or it is set and doesn't yet contain sentry values const originalBaggageHeader = originalHeaders?.['baggage']; if (!originalBaggageHeader || !baggageHeaderHasSentryValues(originalBaggageHeader)) { // From MDN: "If this method is called several times with the same header, the values are merged into one single request header." @@ -447,7 +449,7 @@ function setHeaderOnXhr( } function baggageHeaderHasSentryValues(baggageHeader: string): boolean { - return baggageHeader.split(',').some(value => value.startsWith('sentry-')); + return baggageHeader.split(',').some(value => value.trim().startsWith('sentry-')); } function getFullURL(url: string): string | undefined {