Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://[email protected]/1337',
integrations: [
Sentry.browserTracingIntegration({
idleTimeout: 1000,
onRequestSpanEnd(span, { headers }) {
if (headers) {
span.setAttribute('hook.called.response-type', headers.get('x-response-type'));
}
},
}),
],
tracesSampleRate: 1,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
fetch('http://sentry-test.io/fetch', {
headers: {
foo: 'fetch',
},
});

const xhr = new XMLHttpRequest();

xhr.open('GET', 'http://sentry-test.io/xhr');
xhr.setRequestHeader('foo', 'xhr');
xhr.send();
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { expect } from '@playwright/test';
import type { Event } from '@sentry/core';
import { sentryTest } from '../../../../utils/fixtures';
import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers';

sentryTest('should call onRequestSpanEnd hook', async ({ browserName, getLocalTestUrl, page }) => {
const supportedBrowsers = ['chromium', 'firefox'];

if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
sentryTest.skip();
}

await page.route('http://sentry-test.io/fetch', async route => {
await route.fulfill({
status: 200,
headers: {
'Content-Type': 'application/json',
'X-Response-Type': 'fetch',
'access-control-expose-headers': '*',
},
body: '',
});
});
await page.route('http://sentry-test.io/xhr', async route => {
await route.fulfill({
status: 200,
headers: {
'Content-Type': 'application/json',
'X-Response-Type': 'xhr',
'access-control-expose-headers': '*',
},
body: '',
});
});

const url = await getLocalTestUrl({ testDir: __dirname });

const envelopes = await getMultipleSentryEnvelopeRequests<Event>(page, 2, { url, timeout: 10000 });

const tracingEvent = envelopes[envelopes.length - 1]; // last envelope contains tracing data on all browsers

expect(tracingEvent.spans).toContainEqual(
expect.objectContaining({
op: 'http.client',
data: expect.objectContaining({
type: 'xhr',
'hook.called.response-type': 'xhr',
}),
}),
);

expect(tracingEvent.spans).toContainEqual(
expect.objectContaining({
op: 'http.client',
data: expect.objectContaining({
type: 'fetch',
'hook.called.response-type': 'fetch',
}),
}),
);
});
9 changes: 8 additions & 1 deletion packages/browser-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ export { fetch, setTimeout, clearCachedImplementation, getNativeImplementation }

export { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './instrument/xhr';

export { getBodyString, getFetchRequestArgBody, serializeFormData } from './networkUtils';
export {
getBodyString,
getFetchRequestArgBody,
serializeFormData,
parseXhrResponseHeaders,
getFetchResponseHeaders,
filterAllowedHeaders,
} from './networkUtils';

export { resourceTimingToSpanAttributes } from './metrics/resourceTiming';

Expand Down
55 changes: 55 additions & 0 deletions packages/browser-utils/src/networkUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,58 @@ export function getFetchRequestArgBody(fetchArgs: unknown[] = []): RequestInit['

return (fetchArgs[1] as RequestInit).body;
}

/**
* Parses XMLHttpRequest response headers into a Record.
* Extracted from replay internals to be reusable.
*/
export function parseXhrResponseHeaders(xhr: XMLHttpRequest): Record<string, string> {
const headers = xhr.getAllResponseHeaders();

if (!headers) {
return {};
}

return headers.split('\r\n').reduce((acc: Record<string, string>, line: string) => {
const [key, value] = line.split(': ') as [string, string | undefined];
if (value) {
acc[key.toLowerCase()] = value;
}
return acc;
}, {});
}

/**
* Gets specific headers from a Headers object (Fetch API).
* Extracted from replay internals to be reusable.
*/
export function getFetchResponseHeaders(headers: Headers, allowedHeaders: string[]): Record<string, string> {
const allHeaders: Record<string, string> = {};

allowedHeaders.forEach(header => {
const value = headers.get(header);
if (value) {
allHeaders[header.toLowerCase()] = value;
}
});

return allHeaders;
}

/**
* Filters headers based on an allowed list.
* Extracted from replay internals to be reusable.
*/
export function filterAllowedHeaders(
headers: Record<string, string>,
allowedHeaders: string[],
): Record<string, string> {
return Object.entries(headers).reduce((filteredHeaders: Record<string, string>, [key, value]) => {
const normalizedKey = key.toLowerCase();
// Avoid putting empty strings into the headers
if (allowedHeaders.includes(normalizedKey) && value) {
filteredHeaders[normalizedKey] = value;
}
return filteredHeaders;
}, {});
}
19 changes: 17 additions & 2 deletions packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
/* eslint-disable max-lines */
import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource, WebFetchHeaders } from '@sentry/core';
import type {
Client,
IntegrationFn,
RequestHookInfo,
ResponseHookInfo,
Span,
StartSpanOptions,
TransactionSource,
} from '@sentry/core';
import {
addNonEnumerableProperty,
browserPerformanceTimeOrigin,
Expand Down Expand Up @@ -297,7 +305,12 @@ export interface BrowserTracingOptions {
* You can use it to annotate the span with additional data or attributes, for example by setting
* attributes based on the passed request headers.
*/
onRequestSpanStart?(span: Span, requestInformation: { headers?: WebFetchHeaders }): void;
onRequestSpanStart?(span: Span, requestInformation: RequestHookInfo): void;

/**
* Is called when spans end for outgoing requests, providing access to response headers.
*/
onRequestSpanEnd?(span: Span, responseInformation: ResponseHookInfo): void;
}

const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
Expand Down Expand Up @@ -365,6 +378,7 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
consistentTraceSampling,
enableReportPageLoaded,
onRequestSpanStart,
onRequestSpanEnd,
} = {
...DEFAULT_BROWSER_TRACING_OPTIONS,
...options,
Expand Down Expand Up @@ -692,6 +706,7 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
shouldCreateSpanForRequest,
enableHTTPTimings,
onRequestSpanStart,
onRequestSpanEnd,
});
},
};
Expand Down
61 changes: 27 additions & 34 deletions packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span, WebFetchHeaders } from '@sentry/core';
import type {
Client,
HandlerDataXhr,
RequestHookInfo,
ResponseHookInfo,
SentryWrappedXMLHttpRequest,
Span,
} from '@sentry/core';
import {
addFetchEndInstrumentationHandler,
addFetchInstrumentationHandler,
Expand All @@ -22,11 +29,12 @@ import type { XhrHint } from '@sentry-internal/browser-utils';
import {
addPerformanceInstrumentationHandler,
addXhrInstrumentationHandler,
parseXhrResponseHeaders,
resourceTimingToSpanAttributes,
SENTRY_XHR_DATA_KEY,
} from '@sentry-internal/browser-utils';
import type { BrowserClient } from '../client';
import { WINDOW } from '../helpers';
import { baggageHeaderHasSentryValues, createHeadersSafely, getFullURL, isPerformanceResourceTiming } from './utils';

/** Options for Request Instrumentation */
export interface RequestInstrumentationOptions {
Expand Down Expand Up @@ -102,7 +110,12 @@ export interface RequestInstrumentationOptions {
/**
* Is called when spans are started for outgoing requests.
*/
onRequestSpanStart?(span: Span, requestInformation: { headers?: WebFetchHeaders }): void;
onRequestSpanStart?(span: Span, requestInformation: RequestHookInfo): void;

/**
* Is called when spans end for outgoing requests, providing access to response headers.
*/
onRequestSpanEnd?(span: Span, responseInformation: ResponseHookInfo): void;
}

const responseToSpanId = new WeakMap<object, string>();
Expand All @@ -125,6 +138,7 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
enableHTTPTimings,
tracePropagationTargets,
onRequestSpanStart,
onRequestSpanEnd,
} = {
...defaultRequestInstrumentationOptions,
..._options,
Expand Down Expand Up @@ -171,6 +185,7 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
addFetchInstrumentationHandler(handlerData => {
const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans, {
propagateTraceparent,
onRequestSpanEnd,
});

if (handlerData.response && handlerData.fetchData.__span) {
Expand Down Expand Up @@ -205,34 +220,22 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
shouldAttachHeadersWithTargets,
spans,
propagateTraceparent,
onRequestSpanEnd,
);

if (createdSpan) {
if (enableHTTPTimings) {
addHTTPTimings(createdSpan);
}

let headers;
try {
headers = new Headers(handlerData.xhr.__sentry_xhr_v3__?.request_headers);
} catch {
// noop
}
onRequestSpanStart?.(createdSpan, { headers });
onRequestSpanStart?.(createdSpan, {
headers: createHeadersSafely(handlerData.xhr.__sentry_xhr_v3__?.request_headers),
});
}
});
}
}

function isPerformanceResourceTiming(entry: PerformanceEntry): entry is PerformanceResourceTiming {
return (
entry.entryType === 'resource' &&
'initiatorType' in entry &&
typeof (entry as PerformanceResourceTiming).nextHopProtocol === 'string' &&
(entry.initiatorType === 'fetch' || entry.initiatorType === 'xmlhttprequest')
);
}

/**
* Creates a temporary observer to listen to the next fetch/xhr resourcing timings,
* so that when timings hit their per-browser limit they don't need to be removed.
Expand Down Expand Up @@ -315,6 +318,7 @@ function xhrCallback(
shouldAttachHeaders: (url: string) => boolean,
spans: Record<string, Span>,
propagateTraceparent?: boolean,
onRequestSpanEnd?: RequestInstrumentationOptions['onRequestSpanEnd'],
): Span | undefined {
const xhr = handlerData.xhr;
const sentryXhrData = xhr?.[SENTRY_XHR_DATA_KEY];
Expand All @@ -337,6 +341,10 @@ function xhrCallback(
setHttpStatus(span, sentryXhrData.status_code);
span.end();

onRequestSpanEnd?.(span, {
headers: createHeadersSafely(parseXhrResponseHeaders(xhr as XMLHttpRequest & SentryWrappedXMLHttpRequest)),
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: XHR Memory Leak and Header Parsing Errors

In xhrCallback, XHR spans may leak memory because cleanup and the onRequestSpanEnd callback are skipped if sentryXhrData.status_code is undefined. Additionally, parseXhrResponseHeaders receives a wrapped XHR object, which could lead to runtime errors when parsing response headers for the onRequestSpanEnd callback.

Fix in Cursor Fix in Web

// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete spans[spanId];
}
Expand Down Expand Up @@ -438,18 +446,3 @@ function setHeaderOnXhr(
// 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.trim().startsWith('sentry-'));
}

function getFullURL(url: string): string | undefined {
try {
// By adding a base URL to new URL(), this will also work for relative urls
// If `url` is a full URL, the base URL is ignored anyhow
const parsed = new URL(url, WINDOW.location.origin);
return parsed.href;
} catch {
return undefined;
}
}
46 changes: 46 additions & 0 deletions packages/browser/src/tracing/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { WINDOW } from '../helpers';

/**
* Checks if the baggage header has Sentry values.
*/
export function baggageHeaderHasSentryValues(baggageHeader: string): boolean {
return baggageHeader.split(',').some(value => value.trim().startsWith('sentry-'));
}

/**
* Gets the full URL from a given URL string.
*/
export function getFullURL(url: string): string | undefined {
try {
// By adding a base URL to new URL(), this will also work for relative urls
// If `url` is a full URL, the base URL is ignored anyhow
const parsed = new URL(url, WINDOW.location.origin);
return parsed.href;
} catch {
return undefined;
}
}

/**
* Checks if the entry is a PerformanceResourceTiming.
*/
export function isPerformanceResourceTiming(entry: PerformanceEntry): entry is PerformanceResourceTiming {
return (
entry.entryType === 'resource' &&
'initiatorType' in entry &&
typeof (entry as PerformanceResourceTiming).nextHopProtocol === 'string' &&
(entry.initiatorType === 'fetch' || entry.initiatorType === 'xmlhttprequest')
);
}

/**
* Creates a Headers object from a record of string key-value pairs, and returns undefined if it fails.
*/
export function createHeadersSafely(headers: Record<string, string> | undefined): Headers | undefined {
try {
return new Headers(headers);
} catch {
// noop
return undefined;
}
}
Loading
Loading