Skip to content

Commit a4762fe

Browse files
chargomeclaude
andauthored
fix(orchestrion): Stop force-bundling instrumented deps in dev servers (#23835)
The Vite plugin's `ssr.noExternal` and the Nuxt module's Nitro `externals.inline` force-bundle instrumented packages unconditionally. That's right for a build, but in a dev server it breaks the drivers (`exports is not defined` on Vite, `(0, lodash_1.defaults) is not a function` on Nitro) and takes them off Node's module loader, where the runtime hook `Sentry.init()` registers would have instrumented them instead. Gate both on the build. Dev falls through to the runtime hook and emits the same spans. astro-7 and sveltekit-2 gain a dev pass over their DB tests; the react-router-7 dev skip is removed. Nuxt still records no DB spans in dev — Nitro hoists `ioredis`/`mysql` above `Sentry.init()` in the generated `--import` entry. Tracked separately; this only removes the crash there. Fixes #22857 Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
1 parent a9551bc commit a4762fe

8 files changed

Lines changed: 68 additions & 20 deletions

File tree

‎dev-packages/e2e-tests/test-applications/astro-7/package.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
"astro": "astro",
1010
"start": "node ./dist/server/entry.mjs",
1111
"test:build": "pnpm install && pnpm build",
12-
"test:assert": "TEST_ENV=production playwright test"
12+
"test:assert": "pnpm test:prod && pnpm test:dev",
13+
"test:prod": "TEST_ENV=production playwright test",
14+
"test:dev": "TEST_ENV=development playwright test db"
1315
},
1416
"//": "Need to use ioredis 5.10.1 because that's the last version before they support tracing channels",
1517
"dependencies": {

‎dev-packages/e2e-tests/test-applications/astro-7/playwright.config.mjs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ if (!testEnv) {
66
throw new Error('No test env defined');
77
}
88

9+
// `astro dev` ignores PORT, so the port goes on the command.
910
const config = getPlaywrightConfig({
10-
startCommand: 'pnpm start',
11+
startCommand: testEnv === 'development' ? 'pnpm dev --port 3030' : 'pnpm start',
1112
});
1213

1314
export default {

‎dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@ import { expect, test } from '@playwright/test';
22
import { waitForTransaction } from '@sentry-internal/test-utils';
33
import { APP_NAME } from '../constants';
44

5-
// Orchestrion injects `diagnostics_channel` publishers at build time, so these spans only exist in the
6-
// bundled server build. `react-router dev` serves an unbundled SSR pipeline where the transform never
7-
// runs, so the assertions are meaningless there — skip them in the dev run.
8-
const isDev = process.env.TEST_ENV === 'development';
9-
10-
test.describe('server - orchestrion build-time db instrumentation', () => {
11-
test.skip(isDev, 'orchestrion only injects into the bundled server build, not the dev server');
12-
5+
// Same spans in both runs, from two injectors: the build-time transform in the server bundle, and
6+
// the runtime hook in `react-router dev`, where the drivers stay on Node's own loader.
7+
test.describe('server - orchestrion db instrumentation', () => {
138
test('instruments ioredis automatically via orchestrion', async ({ page }) => {
149
const transactionEventPromise = waitForTransaction(APP_NAME, transactionEvent => {
1510
return (

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
1313
"test:prod": "TEST_ENV=production playwright test",
1414
"test:build": "pnpm install && pnpm build",
15-
"test:assert": "pnpm test:prod"
15+
"test:assert": "pnpm test:prod && pnpm test:dev",
16+
"test:dev": "TEST_ENV=development playwright test db"
1617
},
1718
"//": "Need to use ioredis 5.10.1 because that's the last version before they support tracing channels",
1819
"dependencies": {

‎packages/nuxt/src/vite/orchestrion.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ export function setupOrchestrion(nuxt: Nuxt, hasServerConfig: boolean, buildTime
2626
return;
2727
}
2828

29+
// `nuxt dev` has no bundle to transform, and inlining the CommonJS drivers into Nitro's dev
30+
// server breaks them (`ioredis` throws `(0, lodash_1.defaults) is not a function`).
31+
if (nuxt.options?.dev) {
32+
return;
33+
}
34+
2935
// On Cloudflare (workerd) the SDK is initialized through `sentryCloudflareNitroPlugin` (no
3036
// server config file), so the transform must still run there — detected via the Nitro preset.
3137
// Nitro normalizes preset names, so match any `cloudflare*` spelling.

‎packages/nuxt/test/vite/orchestrion.test.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vites
33

44
const mockSentryOrchestrionPlugin = vi.fn(() => ({ name: 'sentry-orchestrion-plugin' }));
55

6-
function createMockNuxt(options: { _prepare?: boolean } = {}) {
6+
function createMockNuxt(options: { _prepare?: boolean; dev?: boolean } = {}) {
77
const hooks: Record<string, Array<(...args: any[]) => void | Promise<void>>> = {};
88

99
return {
10-
options: { _prepare: options._prepare ?? false },
10+
options: { _prepare: options._prepare ?? false, dev: options.dev ?? false },
1111
hook: (name: string, callback: (...args: any[]) => void | Promise<void>) => {
1212
hooks[name] = hooks[name] || [];
1313
hooks[name].push(callback);
@@ -116,6 +116,19 @@ describe('setupOrchestrion', () => {
116116
expect(nitroConfig).toEqual({});
117117
});
118118

119+
it('does not change Nitro configuration in dev mode', async () => {
120+
const { setupOrchestrion } = await import('../../src/vite/orchestrion');
121+
const mockNuxt = createMockNuxt({ dev: true });
122+
const nitroConfig = {};
123+
124+
setupOrchestrion(mockNuxt as unknown as Nuxt, true);
125+
await mockNuxt.triggerHook('nitro:config', nitroConfig);
126+
127+
// Nothing to transform in dev, and inlining the CommonJS drivers there breaks them.
128+
expect(mockSentryOrchestrionPlugin).not.toHaveBeenCalled();
129+
expect(nitroConfig).toEqual({});
130+
});
131+
119132
it('does not change Nitro configuration in prepare mode', async () => {
120133
const { setupOrchestrion } = await import('../../src/vite/orchestrion');
121134
const mockNuxt = createMockNuxt({ _prepare: true });

‎packages/server-utils/src/orchestrion/bundler/vite.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/vite';
2-
import type { Plugin, ResolvedConfig } from 'vite';
2+
import type { ConfigEnv, Plugin, ResolvedConfig } from 'vite';
33

44
export type { Plugin as VitePlugin } from 'vite';
55
import { instrumentedModuleNames } from '../config';
@@ -83,7 +83,14 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin {
8383
// calls never land in a browser (`client`) bundle (where they'd throw `X is not a function`).
8484
return environment.config.consumer === 'server';
8585
},
86-
config(): { ssr: { noExternal: string[] } } {
86+
config(_config: unknown, env?: ConfigEnv): { ssr: { noExternal: string[] } } | null {
87+
// Vite's dev SSR runner has no CommonJS interop, so an inlined `ioredis`/`mysql` throws
88+
// `exports is not defined` on first import. Left external they stay on Node's loader, where
89+
// the runtime hook `Sentry.init()` registers injects the same publishers.
90+
if (env?.command === 'serve') {
91+
return null;
92+
}
93+
8794
// Force-bundle every instrumented package so the code transform actually
8895
// sees its source. Vite externalizes dependencies in SSR builds by
8996
// default, leaving them as bare `require()`/`import` calls resolved from
@@ -104,6 +111,11 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin {
104111
};
105112
},
106113
configResolved(config: ResolvedConfig): void {
114+
// Nothing is force-bundled in `serve`, so an externalized module is expected there.
115+
if (config.command === 'serve') {
116+
return;
117+
}
118+
107119
// Explicit `ssr.external` string entries take priority over `noExternal`
108120
// in Vite, so they defeat the force-bundling above. (`ssr.external: true`
109121
// does not — `noExternal` entries still win there.)

‎packages/server-utils/test/orchestrion/bundler.test.ts‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,26 @@ describe('sentryOrchestrionWebpackPlugin', () => {
179179
});
180180

181181
describe('sentryOrchestrionPlugin (vite)', () => {
182-
function runConfigResolved(ssrExternal: string[] | true | undefined): ReturnType<typeof vi.fn> {
182+
function runConfigResolved(
183+
ssrExternal: string[] | true | undefined,
184+
command: 'build' | 'serve' = 'build',
185+
): ReturnType<typeof vi.fn> {
183186
const warn = vi.fn();
184187
const plugin = vitePlugin();
185188
(plugin.configResolved as (config: unknown) => void)({
189+
command,
186190
ssr: { external: ssrExternal },
187191
logger: { warn },
188192
} as unknown as ResolvedConfig);
189193
return warn;
190194
}
191195

196+
function runConfig(command: 'build' | 'serve'): { ssr: { noExternal: string[] } } | null {
197+
const plugin = vitePlugin();
198+
const config = plugin.config as (config: unknown, env: unknown) => { ssr: { noExternal: string[] } } | null;
199+
return config.call(plugin, {}, { command, mode: 'production' });
200+
}
201+
192202
it('warns when instrumented modules are listed in ssr.external', () => {
193203
const warn = runConfigResolved(['mysql', 'lodash']);
194204

@@ -204,14 +214,22 @@ describe('sentryOrchestrionPlugin (vite)', () => {
204214
});
205215

206216
it('force-bundles the orchestrion helper package alongside instrumented modules', () => {
207-
const plugin = vitePlugin();
208-
const config = (plugin.config as () => { ssr: { noExternal: string[] } })();
217+
const config = runConfig('build');
209218

210219
// Left external, Vite 5's CommonJS interop turns the snippet's `require`
211220
// into a default import of the named-exports-only ESM entry — a link-time
212221
// crash at server startup.
213-
expect(config.ssr.noExternal).toContain('@sentry/server-utils');
214-
expect(config.ssr.noExternal).toContain('mysql');
222+
expect(config?.ssr.noExternal).toContain('@sentry/server-utils');
223+
expect(config?.ssr.noExternal).toContain('mysql');
224+
});
225+
226+
it('does not force-bundle instrumented modules on the dev server', () => {
227+
// Inlined in dev, the CommonJS drivers throw `exports is not defined` on import.
228+
expect(runConfig('serve')).toBeNull();
229+
});
230+
231+
it('does not warn about externalized instrumented modules on the dev server', () => {
232+
expect(runConfigResolved(['mysql', 'lodash'], 'serve')).not.toHaveBeenCalled();
215233
});
216234

217235
it('gates the transform on the ssr flag (Vite 5 ignores applyToEnvironment)', () => {

0 commit comments

Comments
 (0)