Skip to content

Commit e672a37

Browse files
author
Luca Forstner
committed
Boi am I glad I added tests
1 parent 6a08581 commit e672a37

File tree

5 files changed

+154
-17
lines changed

5 files changed

+154
-17
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
integrations: [Sentry.browserTracingIntegration()],
8+
tracePropagationTargets: ['http://example.com'],
9+
tracesSampleRate: 1,
10+
});

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
fetchPojo.addEventListener('click', () => {
2+
const fetchOptions = {
3+
headers: {
4+
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
5+
baggage: 'sentry-release=4.2.0',
6+
},
7+
};
8+
9+
// Make two fetch requests that reuse the same fetch object
10+
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
11+
fetch('http://example.com/fetch-pojo', fetchOptions)
12+
.then(res => res.text())
13+
.then(() =>
14+
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
15+
),
16+
);
17+
});
18+
19+
fetchArray.addEventListener('click', () => {
20+
const fetchOptions = {
21+
headers: [
22+
['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
23+
['baggage', 'sentry-release=4.2.0'],
24+
],
25+
};
26+
27+
// Make two fetch requests that reuse the same fetch object
28+
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
29+
fetch('http://example.com/fetch-array', fetchOptions)
30+
.then(res => res.text())
31+
.then(() =>
32+
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
33+
),
34+
);
35+
});
36+
37+
fetchHeaders.addEventListener('click', () => {
38+
const fetchOptions = {
39+
headers: new Headers({
40+
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
41+
baggage: 'sentry-release=4.2.0',
42+
}),
43+
};
44+
45+
// Make two fetch requests that reuse the same fetch object
46+
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
47+
fetch('http://example.com/fetch-headers', fetchOptions)
48+
.then(res => res.text())
49+
.then(() =>
50+
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
51+
),
52+
);
53+
});

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="fetchPojo">Fetch POJO</button>
8+
<button id="fetchArray">Fetch array</button>
9+
<button id="fetchHeaders">Fetch Headers</button>
10+
</body>
11+
</html>

‎dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import type { Page, Request } from '@playwright/test';
2+
import { expect } from '@playwright/test';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { shouldSkipTracingTest } from '../../../../utils/helpers';
5+
6+
async function assertRequests({
7+
page,
8+
buttonSelector,
9+
requestMatcher,
10+
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
11+
const requests = await new Promise<Request[]>(resolve => {
12+
const requests: Request[] = [];
13+
page
14+
.route(requestMatcher, (route, request) => {
15+
requests.push(request);
16+
if (requests.length === 2) {
17+
resolve(requests);
18+
}
19+
20+
return route.fulfill({
21+
status: 200,
22+
contentType: 'application/json',
23+
body: JSON.stringify({}),
24+
});
25+
})
26+
.then(() => {
27+
page.click(buttonSelector);
28+
});
29+
});
30+
31+
requests.forEach(request => {
32+
const headers = request.headers();
33+
34+
// No merged sentry trace headers
35+
expect(headers['sentry-trace']).not.toContain(',');
36+
37+
// No multiple baggage entries
38+
expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
39+
});
40+
}
41+
42+
sentryTest(
43+
'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
44+
async ({ getLocalTestUrl, page }) => {
45+
if (shouldSkipTracingTest()) {
46+
sentryTest.skip();
47+
}
48+
49+
const url = await getLocalTestUrl({ testDir: __dirname });
50+
await page.goto(url);
51+
52+
await sentryTest.step('fetch with POJO', () =>
53+
assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
54+
);
55+
56+
await sentryTest.step('fetch with array', () =>
57+
assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
58+
);
59+
60+
await sentryTest.step('fetch with Headers instance', () =>
61+
assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
62+
);
63+
},
64+
);

‎packages/core/src/fetch.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -175,25 +175,24 @@ export function addTracingHeadersToFetchRequest(
175175

176176
return newHeaders as PolymorphicRequestHeaders;
177177
} else if (Array.isArray(headers)) {
178-
const newHeaders = headers
179-
.filter(header => {
178+
const newHeaders = [
179+
...headers
180180
// Remove any existing sentry-trace headers
181-
return !(Array.isArray(header) && header[0] === 'sentry-trace');
182-
})
183-
.map(header => {
184-
if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME) {
185-
return [
186-
BAGGAGE_HEADER_NAME,
187-
...header.map(headerValue =>
188-
typeof headerValue === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerValue) : headerValue,
189-
),
190-
];
191-
} else {
192-
return header;
193-
}
194-
})
181+
.filter(header => {
182+
return !(Array.isArray(header) && header[0] === 'sentry-trace');
183+
})
184+
// Get rid of previous sentry baggage values in baggage header
185+
.map(header => {
186+
if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
187+
const [headerName, headerValue, ...rest] = header;
188+
return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
189+
} else {
190+
return header;
191+
}
192+
}),
195193
// Attach the new sentry-trace header
196-
.concat(['sentry-trace', sentryTraceHeader]);
194+
['sentry-trace', sentryTraceHeader],
195+
];
197196

198197
if (sentryBaggageHeader) {
199198
// If there are multiple entries with the same key, the browser will merge the values into a single request header.

0 commit comments

Comments
 (0)