-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(nextjs): Do not bundle orchestrion on CF WIP #22851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { loadModule } from '@sentry/core'; | ||
| // Type-only imports: erased at compile time so the module graph of the runtime server entry | ||
| // (`index.server.ts` → `withSentryConfig` → …) never statically reaches | ||
| // `@sentry/server-utils/orchestrion/webpack`. That subpath bundles the code-transformer bundler | ||
| // plugin, whose vendored core compiles a WASM lexer at module-evaluation time. On Cloudflare | ||
| // Workers runtime WASM compilation is forbidden, so a static import made every cold start throw a | ||
| // `CompileError`; elsewhere it was silent bundle weight. See #22794. | ||
| import type { | ||
| getOrchestrionLoaderPath, | ||
| getSentryInstrumentations, | ||
| resolveOrchestrionRuntimeRequest, | ||
| sentryOrchestrionWebpackPlugin, | ||
| serializeInstrumentations, | ||
| } from '@sentry/server-utils/orchestrion/webpack'; | ||
|
|
||
| type OrchestrionWebpackModule = { | ||
| getOrchestrionLoaderPath: typeof getOrchestrionLoaderPath; | ||
| getSentryInstrumentations: typeof getSentryInstrumentations; | ||
| resolveOrchestrionRuntimeRequest: typeof resolveOrchestrionRuntimeRequest; | ||
| sentryOrchestrionWebpackPlugin: typeof sentryOrchestrionWebpackPlugin; | ||
| serializeInstrumentations: typeof serializeInstrumentations; | ||
| }; | ||
|
|
||
| /** | ||
| * Loads `@sentry/server-utils/orchestrion/webpack` at build time via a runtime require rather than | ||
| * a static import. This mirrors how the Sentry sourcemap plugin (`@sentry/bundler-plugins/webpack`) | ||
| * is loaded in `webpack.ts` — both are build-time-only and must stay out of the runtime server | ||
| * bundle's static module graph. | ||
| * | ||
| * Callers run exclusively inside `withSentryConfig`'s bundler-config functions (never at module | ||
| * scope), so the deferred load resolves the same way the static import did during a real build. | ||
| */ | ||
| export function loadOrchestrionBundlerPlugin(): OrchestrionWebpackModule | undefined { | ||
| return loadModule<OrchestrionWebpackModule>('@sentry/server-utils/orchestrion/webpack', module); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { readdirSync, readFileSync } from 'node:fs'; | ||
| import { join, relative, resolve } from 'node:path'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| /** | ||
| * `@sentry/server-utils/orchestrion/webpack` bundles the code-transformer bundler plugin, whose | ||
| * vendored core compiles a WASM lexer at module-evaluation time. A static import of this subpath | ||
| * anywhere in the package pulls that module into the runtime server entry's static graph, so on | ||
| * Cloudflare Workers — where runtime WASM compilation is forbidden — every cold start threw a | ||
| * `CompileError`, even with the feature disabled. It must only ever be reached through the deferred | ||
| * `loadModule(...)` in `loadOrchestrionBundlerPlugin`. | ||
| * | ||
| * Regression test for https://github.com/getsentry/sentry-javascript/issues/22794 | ||
| */ | ||
| describe('`@sentry/server-utils/orchestrion/webpack` is never a static import in the built package', () => { | ||
| const builds = { | ||
| cjs: resolve(__dirname, '../build/cjs'), | ||
| esm: resolve(__dirname, '../build/esm'), | ||
| }; | ||
|
|
||
| const SUBPATH = '@sentry/server-utils/orchestrion/webpack'; | ||
|
|
||
| // Static ESM `import`/`export … from '<subpath>'` (named, namespace, or bare side-effect import). | ||
| const STATIC_IMPORT = new RegExp(String.raw`\b(?:import|export)\b[^;]*?from\s*['"]${escapeRegExp(SUBPATH)}['"]`); | ||
| const BARE_IMPORT = new RegExp(String.raw`\bimport\s*['"]${escapeRegExp(SUBPATH)}['"]`); | ||
| // Static CJS `require('<subpath>')` — distinct from the deferred `loadModule('<subpath>', module)`. | ||
| const STATIC_REQUIRE = new RegExp(String.raw`\brequire\(\s*['"]${escapeRegExp(SUBPATH)}['"]`); | ||
|
|
||
| it.each(Object.keys(builds))('has no static import of the orchestrion webpack subpath in the %s build', build => { | ||
| const root = builds[build as keyof typeof builds]; | ||
|
|
||
| const offenders = jsFiles(root) | ||
| .filter(file => { | ||
| const source = readFileSync(file, 'utf8'); | ||
| return STATIC_IMPORT.test(source) || BARE_IMPORT.test(source) || STATIC_REQUIRE.test(source); | ||
| }) | ||
| .map(file => relative(root, file)); | ||
|
|
||
| expect(offenders).toEqual([]); | ||
| }); | ||
|
|
||
| it('reaches the orchestrion webpack subpath only through the deferred loader', () => { | ||
| for (const [, root] of Object.entries(builds)) { | ||
| const referencing = jsFiles(root).filter(file => readFileSync(file, 'utf8').includes(SUBPATH)); | ||
| expect(referencing.map(file => relative(root, file))).toEqual([ | ||
| join('config', 'loadOrchestrionBundlerPlugin.js'), | ||
| ]); | ||
| expect(readFileSync(referencing[0]!, 'utf8')).toMatch( | ||
| new RegExp(String.raw`loadModule\(\s*['"]${escapeRegExp(SUBPATH)}['"]\s*,\s*module\s*\)`), | ||
| ); | ||
| } | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test loop should use it.eachLow Severity The second regression case loops over the CJS and ESM builds inside one Triggered by project rule: PR Review Guidelines for Cursor Bot Reviewed by Cursor Bugbot for commit 1e6f94b. Configure here. |
||
| }); | ||
|
|
||
| function jsFiles(dir: string): string[] { | ||
| const files: string[] = []; | ||
| for (const entry of readdirSync(dir, { withFileTypes: true })) { | ||
| const full = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| files.push(...jsFiles(full)); | ||
| } else if (entry.isFile() && entry.name.endsWith('.js')) { | ||
| files.push(full); | ||
| } | ||
| } | ||
| return files; | ||
| } | ||
|
|
||
| function escapeRegExp(value: string): string { | ||
| return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The dynamic import for the orchestrion bundler plugin can fail silently, deactivating the
useDiagnosticsChannelInjectionfeature without any warning to the user.Severity: LOW
Suggested Fix
Add a warning or log message when
loadOrchestrionBundlerPlugin()returnsundefinedinside the webpack configuration. This would alert the user that theuseDiagnosticsChannelInjectionfeature they enabled could not be activated due to a module loading failure.Prompt for AI Agent
Also affects:
packages/nextjs/src/config/turbopack/constructTurbopackConfig.ts:140~140packages/nextjs/src/config/loadOrchestrionBundlerPlugin.ts:28~34Did we get this right? 👍 / 👎 to inform future reviews.