Skip to content

Commit d9b7543

Browse files
authored
feat(sveltekit): Only inject fetch proxy script for SvelteKit < 2.16.0 (#15126)
With v9 onwards, we'll only inject the fetch proxy script into SvelteKit versions where we actually need it. This is all kit versions below 2.16.0. I'll update docs in a follow-up PR. fixes #15007
1 parent 829d4da commit d9b7543

26 files changed

+91
-16
lines changed

dev-packages/e2e-tests/test-applications/sveltekit-2-twp/package.json renamed to dev-packages/e2e-tests/test-applications/sveltekit-2.5.0-twp/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"name": "sveltekit-2-svelte-5",
2+
"name": "sveltekit-2.5.0-twp",
33
"version": "0.0.1",
44
"private": true,
55
"scripts": {
@@ -22,7 +22,7 @@
2222
"@sentry-internal/test-utils": "link:../../../test-utils",
2323
"@sentry/core": "latest || *",
2424
"@sveltejs/adapter-auto": "^3.0.0",
25-
"@sveltejs/kit": "^2.0.0",
25+
"@sveltejs/kit": "2.5.0",
2626
"@sveltejs/vite-plugin-svelte": "^3.0.0",
2727
"svelte": "^5.0.0-next.115",
2828
"svelte-check": "^3.6.0",

dev-packages/e2e-tests/test-applications/sveltekit-2-twp/start-event-proxy.mjs renamed to dev-packages/e2e-tests/test-applications/sveltekit-2.5.0-twp/start-event-proxy.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ import { startEventProxyServer } from '@sentry-internal/test-utils';
22

33
startEventProxyServer({
44
port: 3031,
5-
proxyServerName: 'sveltekit-2-twp',
5+
proxyServerName: 'sveltekit-2.5.0-twp',
66
});

dev-packages/e2e-tests/test-applications/sveltekit-2-twp/tests/errors.test.ts renamed to dev-packages/e2e-tests/test-applications/sveltekit-2.5.0-twp/tests/errors.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ import { expect, test } from '@playwright/test';
22
import { waitForError } from '@sentry-internal/test-utils';
33

44
test('errors on frontend and backend are connected by the same trace', async ({ page }) => {
5-
const clientErrorPromise = waitForError('sveltekit-2-twp', evt => {
5+
const clientErrorPromise = waitForError('sveltekit-2.5.0-twp', evt => {
66
return evt.exception?.values?.[0].value === 'Client Error';
77
});
88

9-
const serverErrorPromise = waitForError('sveltekit-2-twp', evt => {
9+
const serverErrorPromise = waitForError('sveltekit-2.5.0-twp', evt => {
1010
return evt.exception?.values?.[0].value === 'No search query provided';
1111
});
1212

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { expect, test } from '@playwright/test';
2+
3+
test.describe('SDK-internal behavior', () => {
4+
test('Injects fetch proxy script for SvelteKit<2.16.0', async ({ page }) => {
5+
await page.goto('/');
6+
7+
const sentryCarrier = await page.evaluate('typeof window.__SENTRY__');
8+
const proxyHandle = await page.evaluate('typeof window._sentryFetchProxy');
9+
10+
// sanity check
11+
expect(sentryCarrier).toBe('object');
12+
13+
// fetch proxy script ran
14+
expect(proxyHandle).toBe('function');
15+
});
16+
});

dev-packages/e2e-tests/test-applications/sveltekit-2/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"@sentry/core": "latest || *",
2424
"@sveltejs/adapter-auto": "^3.0.0",
2525
"@sveltejs/adapter-node": "^2.0.0",
26-
"@sveltejs/kit": "^2.5.0",
26+
"@sveltejs/kit": "^2.16.0",
2727
"@sveltejs/vite-plugin-svelte": "^3.0.0",
2828
"svelte": "^4.2.8",
2929
"svelte-check": "^3.6.0",
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForInitialPageload } from './utils';
3+
4+
test.describe('SDK-internal behavior', () => {
5+
test("Doesn't inject fetch proxy script for SvelteKit>=2.16.0", async ({ page }) => {
6+
await waitForInitialPageload(page, { route: '/' });
7+
const sentryCarrier = await page.evaluate('typeof window.__SENTRY__');
8+
const proxyHandle = await page.evaluate('typeof window._sentryFetchProxy');
9+
10+
// sanity check
11+
expect(sentryCarrier).toBe('object');
12+
13+
// fetch proxy script didn't run
14+
expect(proxyHandle).toBe('undefined');
15+
});
16+
});

packages/sveltekit/package.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99
"engines": {
1010
"node": ">=18"
1111
},
12-
"files": [
13-
"/build"
14-
],
12+
"files": ["/build"],
1513
"main": "build/cjs/index.server.js",
1614
"module": "build/esm/index.server.js",
1715
"browser": "build/esm/index.client.js",

packages/sveltekit/src/server/handle.ts

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ export function addSentryCodeToPage(options: { injectFetchProxyScript: boolean }
9393
* ```
9494
*/
9595
export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
96-
const options: Required<SentryHandleOptions> = {
97-
handleUnknownRoutes: false,
98-
injectFetchProxyScript: true,
99-
...handlerOptions,
96+
const { handleUnknownRoutes, ...rest } = handlerOptions ?? {};
97+
const options = {
98+
handleUnknownRoutes: handleUnknownRoutes ?? false,
99+
...rest,
100100
};
101101

102102
const sentryRequestHandler: Handle = input => {
@@ -131,12 +131,24 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
131131

132132
async function instrumentHandle(
133133
{ event, resolve }: Parameters<Handle>[0],
134-
options: Required<SentryHandleOptions>,
134+
options: SentryHandleOptions,
135135
): Promise<Response> {
136136
if (!event.route?.id && !options.handleUnknownRoutes) {
137137
return resolve(event);
138138
}
139139

140+
// caching the result of the version check in `options.injectFetchProxyScript`
141+
// to avoid doing the dynamic import on every request
142+
if (options.injectFetchProxyScript == null) {
143+
try {
144+
// @ts-expect-error - the dynamic import is fine here
145+
const { VERSION } = await import('@sveltejs/kit');
146+
options.injectFetchProxyScript = isFetchProxyRequired(VERSION);
147+
} catch {
148+
options.injectFetchProxyScript = true;
149+
}
150+
}
151+
140152
const routeName = `${event.request.method} ${event.route?.id || event.url.pathname}`;
141153

142154
if (getIsolationScope() !== getDefaultIsolationScope()) {
@@ -161,7 +173,7 @@ async function instrumentHandle(
161173
normalizedRequest: winterCGRequestToRequestData(event.request.clone()),
162174
});
163175
const res = await resolve(event, {
164-
transformPageChunk: addSentryCodeToPage({ injectFetchProxyScript: options.injectFetchProxyScript }),
176+
transformPageChunk: addSentryCodeToPage({ injectFetchProxyScript: options.injectFetchProxyScript ?? true }),
165177
});
166178
if (span) {
167179
setHttpStatus(span, res.status);
@@ -177,3 +189,19 @@ async function instrumentHandle(
177189
await flushIfServerless();
178190
}
179191
}
192+
193+
/**
194+
* We only need to inject the fetch proxy script for SvelteKit versions < 2.16.0.
195+
* Exported only for testing.
196+
*/
197+
export function isFetchProxyRequired(version: string): boolean {
198+
try {
199+
const [major, minor] = version.trim().replace(/-.*/, '').split('.').map(Number);
200+
if (major != null && minor != null && (major > 2 || (major === 2 && minor >= 16))) {
201+
return false;
202+
}
203+
} catch {
204+
// ignore
205+
}
206+
return true;
207+
}

packages/sveltekit/test/server/handle.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import type { Handle } from '@sveltejs/kit';
1414
import { redirect } from '@sveltejs/kit';
1515
import { vi } from 'vitest';
1616

17-
import { FETCH_PROXY_SCRIPT, addSentryCodeToPage, sentryHandle } from '../../src/server/handle';
17+
import { FETCH_PROXY_SCRIPT, addSentryCodeToPage, isFetchProxyRequired, sentryHandle } from '../../src/server/handle';
1818
import { getDefaultNodeClientOptions } from '../utils';
1919

2020
const mockCaptureException = vi.spyOn(SentryNode, 'captureException').mockImplementation(() => 'xx');
@@ -462,3 +462,20 @@ describe('addSentryCodeToPage', () => {
462462
expect(transformed).not.toContain(`<script >${FETCH_PROXY_SCRIPT}</script>`);
463463
});
464464
});
465+
466+
describe('isFetchProxyRequired', () => {
467+
it.each(['2.16.0', '2.16.1', '2.17.0', '3.0.0', '3.0.0-alpha.0'])(
468+
'returns false if the version is greater than or equal to 2.16.0 (%s)',
469+
version => {
470+
expect(isFetchProxyRequired(version)).toBe(false);
471+
},
472+
);
473+
474+
it.each(['2.15.0', '2.15.1', '1.30.0', '1.0.0'])('returns true if the version is lower than 2.16.0 (%s)', version => {
475+
expect(isFetchProxyRequired(version)).toBe(true);
476+
});
477+
478+
it.each(['invalid', 'a.b.c'])('returns true for an invalid version (%s)', version => {
479+
expect(isFetchProxyRequired(version)).toBe(true);
480+
});
481+
});

0 commit comments

Comments
 (0)