Skip to content
Open
Show file tree
Hide file tree
Changes from all 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 @@
<h1>Prerendered page</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const prerender = true;
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,17 @@ test.describe('SDK-internal behavior', () => {
// fetch proxy script didn't run
expect(proxyHandle).toBe('undefined');
});

test("Doesn't add trace meta tags to prerendered pages", async ({ baseURL }) => {
const prerenderedHtml = await (await fetch(`${baseURL}/prerendered`)).text();
const ssrHtml = await (await fetch(`${baseURL}/`)).text();

expect(prerenderedHtml).toContain('Prerendered page');
expect(prerenderedHtml).not.toContain('<meta name="sentry-trace"');
expect(prerenderedHtml).not.toContain('<meta name="baggage"');

// SSR pages still get the meta tags
expect(ssrHtml).toContain('<meta name="sentry-trace"');
expect(ssrHtml).toContain('<meta name="baggage"');
});
});
16 changes: 15 additions & 1 deletion packages/sveltekit/src/server-common/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ export function addSentryCodeToPage(options: {
injectFetchProxyScript: boolean;
}): NonNullable<ResolveOptions['transformPageChunk']> {
return ({ html }) => {
const metaTags = getTraceMetaTags();
// Pages rendered at build time (prerendering) would bake one trace id and sampling
// decision into the HTML for every visitor, so we skip the meta tags there.
const metaTags = isSvelteKitBuilding() ? '' : getTraceMetaTags();
const headWithMetaTags = metaTags ? `<head>\n${metaTags}` : '<head>';

const headWithFetchScript = options.injectFetchProxyScript ? `\n<script>${FETCH_PROXY_SCRIPT}</script>` : '';
Expand All @@ -88,6 +90,18 @@ export function addSentryCodeToPage(options: {
};
}

/**
* `building` from `$app/environment` isn't available here (the SDK is loaded from node_modules at
* runtime), so we rely on env vars that SvelteKit copies into the worker it prerenders in:
* - `_SENTRY_SVELTEKIT_BUILDING`: set by our Vite plugin during `vite build`
* - `SVELTEKIT_FORK`: set by SvelteKit itself, covers apps without our Vite plugin
*/
function isSvelteKitBuilding(): boolean {
// `process` doesn't exist in every runtime (e.g. Cloudflare Workers without `nodejs_compat`)
const env = typeof process !== 'undefined' ? process.env : undefined;
return !!(env?._SENTRY_SVELTEKIT_BUILDING || env?.SVELTEKIT_FORK);
}

/**
* We only need to inject the fetch proxy script for SvelteKit versions < 2.16.0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The check for the undocumented SVELTEKIT_FORK environment variable can incorrectly return true in production on some platforms (e.g., Netlify), silently disabling trace meta tag injection for SSR pages.
Severity: MEDIUM

Suggested Fix

Avoid relying on the undocumented SVELTEKIT_FORK environment variable as it can be present in production environments. Instead, find a more reliable, documented way to detect prerendering that isn't ambiguous with production environments. Alternatively, add more context checks, such as verifying if the code is running in a worker_thread, to ensure it's truly a prerendering worker and not a production server.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/sveltekit/src/server-common/handle.ts#L106

Potential issue: The function `isSvelteKitBuilding` checks for the
`_SENTRY_SVELTEKIT_BUILDING` or `SVELTEKIT_FORK` environment variables to determine if
it's a build-time prerendering phase. However, the undocumented `SVELTEKIT_FORK`
variable can also be present in production runtime environments on certain hosting
platforms like Netlify. When this occurs during a standard server-side render (SSR)
request in production, `isSvelteKitBuilding` will incorrectly return `true`. This
prevents the injection of Sentry trace meta tags into the HTML, which silently breaks
distributed tracing functionality for all dynamically rendered pages on affected
platforms.

Did we get this right? 👍 / 👎 to inform future reviews.

* Exported only for testing.
Expand Down
16 changes: 16 additions & 0 deletions packages/sveltekit/src/vite/sentryVitePlugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}
sentryOrchestrionPlugin({
buildTimeInstrumentation: mergedOptions.buildTimeInstrumentation,
}),
makeBuildFlagPlugin(),
);

const sentryVitePluginsOptions = generateVitePluginOptions(mergedOptions);
Expand Down Expand Up @@ -108,6 +109,21 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {}
return sentryPlugins;
}

/**
* Flags `vite build` so `sentryHandle` can skip trace meta tags on prerendered pages.
* SvelteKit prerenders in a worker that inherits `process.env`, while the production
* server runs in a separate process that never sees this flag.
*/
function makeBuildFlagPlugin(): Plugin {
return {
name: 'sentry-sveltekit-build-flag',
apply: 'build',
config() {
process.env._SENTRY_SVELTEKIT_BUILDING = 'true';
},
};
}

// A bare subpath (not a relative import) so this plugin's `resolveId` can intercept it.
const BROWSER_TRACING_VARIANT_ID = '@sentry/sveltekit/browser-tracing-variant';

Expand Down
23 changes: 22 additions & 1 deletion packages/sveltekit/test/server-common/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import * as SentryCore from '@sentry/core';
import { NodeClient, setCurrentClient } from '@sentry/node';
import type { Handle } from '@sveltejs/kit';
import { redirect } from '@sveltejs/kit';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import {
addSentryCodeToPage,
FETCH_PROXY_SCRIPT,
Expand Down Expand Up @@ -514,6 +514,27 @@ describe('addSentryCodeToPage', () => {
expect(transformed).toContain('<meta name="baggage"');
expect(transformed).not.toContain(`<script >${FETCH_PROXY_SCRIPT}</script>`);
});

describe('while SvelteKit builds the app', () => {
afterEach(() => {
vi.unstubAllEnvs();
});

// `_SENTRY_SVELTEKIT_BUILDING` is set by our Vite plugin, `SVELTEKIT_FORK` by SvelteKit's prerender worker
it.each(['_SENTRY_SVELTEKIT_BUILDING', 'SVELTEKIT_FORK'])(
"doesn't add meta tags but still adds the fetch proxy script if %s is set",
envVar => {
vi.stubEnv(envVar, 'true');

const transformPageChunk = addSentryCodeToPage({ injectFetchProxyScript: true });
const transformed = transformPageChunk({ html, done: true }) as string;

expect(transformed).not.toContain('<meta name="sentry-trace"');
expect(transformed).not.toContain('<meta name="baggage"');
expect(transformed).toContain(`<script>${FETCH_PROXY_SCRIPT}</script>`);
},
);
});
});

describe('isFetchProxyRequired', () => {
Expand Down
40 changes: 34 additions & 6 deletions packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import type { Plugin } from 'vite';
import { describe, expect, it, vi } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import * as autoInstrument from '../../src/vite/autoInstrument';
import { generateVitePluginOptions, sentrySvelteKit } from '../../src/vite/sentryVitePlugins';
import * as sourceMaps from '../../src/vite/sourceMaps';
Expand Down Expand Up @@ -69,9 +69,9 @@ describe('sentrySvelteKit()', () => {
expect(plugins).toBeInstanceOf(Array);
// 1 kit config resolver + 1 browser-tracing variant resolver + 1 OpenTelemetry API resolver
// + 1 auto instrument plugin
// + 1 orchestrion plugin + 1 global values injection plugin + 1 modified main plugin
// + 1 orchestrion plugin + 1 build flag plugin + 1 global values injection plugin + 1 modified main plugin
// + 3 custom plugins
expect(plugins).toHaveLength(10);
expect(plugins).toHaveLength(11);
});

it('returns the custom sentry source maps upload plugin, unmodified sourcemaps plugins and the auto-instrument plugin by default', async () => {
Expand All @@ -88,6 +88,8 @@ describe('sentrySvelteKit()', () => {
'sentry-auto-instrumentation',
// orchestrion build-time instrumentation plugin:
'sentry-orchestrion-vite',
// build flag plugin:
'sentry-sveltekit-build-flag',
// global values injection plugin:
'sentry-sveltekit-global-values-injection-plugin',
// modified main plugin (writeBundle deferred to closeBundle):
Expand All @@ -101,7 +103,7 @@ describe('sentrySvelteKit()', () => {

it("doesn't return the sentry source maps plugins if autoUploadSourcemaps is `false`", async () => {
const plugins = await getSentrySvelteKitPlugins({ autoUploadSourceMaps: false });
expect(plugins).toHaveLength(5); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + auto instrument + orchestrion
expect(plugins).toHaveLength(6); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + auto instrument + orchestrion + build flag
});

it("doesn't return the sentry source maps plugins if `NODE_ENV` is development", async () => {
Expand All @@ -111,7 +113,7 @@ describe('sentrySvelteKit()', () => {
const plugins = await getSentrySvelteKitPlugins({ autoUploadSourceMaps: true, autoInstrument: true });
const instrumentPlugin = plugins[3];

expect(plugins).toHaveLength(6); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + auto instrument + orchestrion + global values injection
expect(plugins).toHaveLength(7); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + auto instrument + orchestrion + build flag + global values injection
expect(instrumentPlugin?.name).toEqual('sentry-auto-instrumentation');

process.env.NODE_ENV = previousEnv;
Expand All @@ -120,7 +122,7 @@ describe('sentrySvelteKit()', () => {
it("doesn't return the auto instrument plugin if autoInstrument is `false`", async () => {
const plugins = await getSentrySvelteKitPlugins({ autoInstrument: false });
const pluginNames = plugins.map(plugin => plugin.name);
expect(plugins).toHaveLength(9); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + orchestrion + global values injection + 1 modified main plugin + 3 custom plugins
expect(plugins).toHaveLength(10); // kit config resolver + browser-tracing variant resolver + OpenTelemetry API resolver + orchestrion + build flag + global values injection + 1 modified main plugin + 3 custom plugins
expect(pluginNames).not.toContain('sentry-auto-instrumentation');
});

Expand Down Expand Up @@ -293,6 +295,32 @@ describe('OpenTelemetry API resolver plugin', () => {
});
});

describe('build flag plugin', () => {
afterEach(() => {
vi.unstubAllEnvs();
});

async function getBuildFlagPlugin() {
const plugins = await getSentrySvelteKitPlugins({ autoUploadSourceMaps: false });
return plugins.find(p => p.name === 'sentry-sveltekit-build-flag')!;
}

it('only applies to builds', async () => {
const plugin = await getBuildFlagPlugin();
expect(plugin?.apply).toBe('build');
});

it('sets `_SENTRY_SVELTEKIT_BUILDING` so the prerender worker inherits it', async () => {
vi.stubEnv('_SENTRY_SVELTEKIT_BUILDING', undefined);
const plugin = await getBuildFlagPlugin();

// @ts-expect-error - hook is a plain function here and doesn't need a plugin context
plugin?.config?.({}, { command: 'build', mode: 'production' });

expect(process.env._SENTRY_SVELTEKIT_BUILDING).toBe('true');
});
});

describe('generateVitePluginOptions', () => {
it('returns null if no relevant options are provided', () => {
const options: SentrySvelteKitPluginOptions = {};
Expand Down