diff --git a/.size-limit.js b/.size-limit.js index a244ccb2a2a3..bdfe8a4397e2 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -79,7 +79,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'), gzip: true, - limit: '78 KB', + limit: '78.1 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback)', @@ -138,7 +138,7 @@ module.exports = [ import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'), ignore: ['react/jsx-runtime'], gzip: true, - limit: '39 KB', + limit: '39.05 KB', }, // Vue SDK (ESM) { diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js new file mode 100644 index 000000000000..7cd076a052e5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [Sentry.browserTracingIntegration()], + tracePropagationTargets: ['http://example.com'], + tracesSampleRate: 1, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js new file mode 100644 index 000000000000..5951d6d33411 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js @@ -0,0 +1,53 @@ +fetchPojo.addEventListener('click', () => { + const fetchOptions = { + headers: { + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=4.2.0', + }, + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-pojo', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)), + ), + ); +}); + +fetchArray.addEventListener('click', () => { + const fetchOptions = { + headers: [ + ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'], + ['baggage', 'sentry-release=4.2.0'], + ], + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-array', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)), + ), + ); +}); + +fetchHeaders.addEventListener('click', () => { + const fetchOptions = { + headers: new Headers({ + 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', + baggage: 'sentry-release=4.2.0', + }), + }; + + // Make two fetch requests that reuse the same fetch object + Sentry.startSpan({ name: 'does-not-matter-1' }, () => + fetch('http://example.com/fetch-headers', fetchOptions) + .then(res => res.text()) + .then(() => + Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)), + ), + ); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html new file mode 100644 index 000000000000..dc60d6d83808 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts new file mode 100644 index 000000000000..5ae2f83924d0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts @@ -0,0 +1,64 @@ +import type { Page, Request } from '@playwright/test'; +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../../utils/fixtures'; +import { shouldSkipTracingTest } from '../../../../utils/helpers'; + +async function assertRequests({ + page, + buttonSelector, + requestMatcher, +}: { page: Page; buttonSelector: string; requestMatcher: string }) { + const requests = await new Promise(resolve => { + const requests: Request[] = []; + page + .route(requestMatcher, (route, request) => { + requests.push(request); + if (requests.length === 2) { + resolve(requests); + } + + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}), + }); + }) + .then(() => { + page.click(buttonSelector); + }); + }); + + requests.forEach(request => { + const headers = request.headers(); + + // No merged sentry trace headers + expect(headers['sentry-trace']).not.toContain(','); + + // No multiple baggage entries + expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1); + }); +} + +sentryTest( + 'Ensure the SDK does not infinitely append tracing headers to outgoing requests', + async ({ getLocalTestUrl, page }) => { + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } + + const url = await getLocalTestUrl({ testDir: __dirname }); + await page.goto(url); + + await sentryTest.step('fetch with POJO', () => + assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }), + ); + + await sentryTest.step('fetch with array', () => + assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }), + ); + + await sentryTest.step('fetch with Headers instance', () => + assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }), + ); + }, +); diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 9b9d2ece836b..26a993ff0e12 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -1,6 +1,7 @@ import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types'; import { BAGGAGE_HEADER_NAME, + SENTRY_BAGGAGE_KEY_PREFIX, dynamicSamplingContextToSentryBaggageHeader, generateSentryTraceHeader, isInstanceOf, @@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest( request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package, client: Client, scope: Scope, - options: { + fetchOptionsObj: { headers?: | { [key: string]: string[] | string | undefined; @@ -145,7 +146,7 @@ export function addTracingHeadersToFetchRequest( ); const headers = - options.headers || + fetchOptionsObj.headers || (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined); if (!headers) { @@ -153,17 +154,45 @@ export function addTracingHeadersToFetchRequest( } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) { const newHeaders = new Headers(headers as Headers); - newHeaders.append('sentry-trace', sentryTraceHeader); + newHeaders.set('sentry-trace', sentryTraceHeader); if (sentryBaggageHeader) { - // If the same header is appended multiple times the browser will merge the values into a single request header. - // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header. - newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME); + if (prevBaggageHeader) { + const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader); + newHeaders.set( + BAGGAGE_HEADER_NAME, + // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header + // otherwise just set the sentry baggage header + prevHeaderStrippedFromSentryBaggage + ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}` + : sentryBaggageHeader, + ); + } else { + newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader); + } } return newHeaders as PolymorphicRequestHeaders; } else if (Array.isArray(headers)) { - const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]]; + const newHeaders = [ + ...headers + // Remove any existing sentry-trace headers + .filter(header => { + return !(Array.isArray(header) && header[0] === 'sentry-trace'); + }) + // Get rid of previous sentry baggage values in baggage header + .map(header => { + if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') { + const [headerName, headerValue, ...rest] = header; + return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest]; + } else { + return header; + } + }), + // Attach the new sentry-trace header + ['sentry-trace', sentryTraceHeader], + ]; if (sentryBaggageHeader) { // If there are multiple entries with the same key, the browser will merge the values into a single request header. @@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest( return newHeaders as PolymorphicRequestHeaders; } else { const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined; - const newBaggageHeaders: string[] = []; + let newBaggageHeaders: string[] = []; if (Array.isArray(existingBaggageHeader)) { - newBaggageHeaders.push(...existingBaggageHeader); + newBaggageHeaders = existingBaggageHeader + .map(headerItem => + typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem, + ) + .filter(headerItem => headerItem === ''); } else if (existingBaggageHeader) { - newBaggageHeaders.push(existingBaggageHeader); + newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader)); } if (sentryBaggageHeader) { @@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void { } span.end(); } + +function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string { + return ( + baggageHeader + .split(',') + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) + .join(',') + ); +}