Skip to content
Draft
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
4 changes: 2 additions & 2 deletions packages/nextjs/src/config/diagnosticsChannelInjection.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { resolveOrchestrionRuntimeRequest } from '@sentry/server-utils/orchestrion/webpack';
import { loadOrchestrionBundlerPlugin } from './loadOrchestrionBundlerPlugin';

/**
* Instrumented packages verified (via e2e) to bundle correctly, removed from Sentry's own
Expand Down Expand Up @@ -58,6 +58,6 @@ export async function externalizeOrchestrionRuntimePackages({
return undefined;
}

const resolved = resolveOrchestrionRuntimeRequest(request);
const resolved = loadOrchestrionBundlerPlugin()?.resolveOrchestrionRuntimeRequest(request);
return resolved ? `commonjs ${resolved}` : undefined;
}
35 changes: 35 additions & 0 deletions packages/nextjs/src/config/loadOrchestrionBundlerPlugin.ts
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
@@ -1,10 +1,6 @@
import { debug } from '@sentry/core';
import * as path from 'path';
import {
getOrchestrionLoaderPath,
getSentryInstrumentations,
serializeInstrumentations,
} from '@sentry/server-utils/orchestrion/webpack';
import { loadOrchestrionBundlerPlugin } from '../loadOrchestrionBundlerPlugin';
import type { VercelCronsConfig } from '../../common/types';
import type { RouteManifest } from '../manifest/types';
import type {
Expand Down Expand Up @@ -138,16 +134,23 @@ function maybeAddOrchestrionRule(
return rules;
}

const orchestrion = loadOrchestrionBundlerPlugin();
if (!orchestrion) {
return rules;
}

return safelyAddTurbopackRule(rules, {
matcher: '*.{js,mjs,cjs}',
rule: {
condition: 'node',
loaders: [
{
loader: getOrchestrionLoaderPath(),
loader: orchestrion.getOrchestrionLoaderPath(),
// Turbopack JSON-serializes loader options, so a RegExp `filePath` must be encoded first.
options: {
instrumentations: serializeInstrumentations(getSentryInstrumentations()) as unknown as JSONValue[],
instrumentations: orchestrion.serializeInstrumentations(
orchestrion.getSentryInstrumentations(),
) as unknown as JSONValue[],
},
},
],
Expand Down
9 changes: 6 additions & 3 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {
WebpackEntryProperty,
WebpackPluginInstance,
} from './types';
import { sentryOrchestrionWebpackPlugin } from '@sentry/server-utils/orchestrion/webpack';
import { loadOrchestrionBundlerPlugin } from './loadOrchestrionBundlerPlugin';
import { getNextjsVersion, getPackageModules } from './util';
import type { VercelCronsConfigResult } from './withSentryConfig/getFinalConfigObjectUtils';

Expand Down Expand Up @@ -430,8 +430,11 @@ export function constructWebpackConfigFunction({

// Orchestrion code-transform loader — Node server runtime only, never the edge compilation
if (runtime === 'server' && userSentryOptions._experimental?.useDiagnosticsChannelInjection) {
newConfig.plugins.push(sentryOrchestrionWebpackPlugin() as unknown as WebpackPluginInstance);
prependOrchestrionRuntimeExternals(newConfig);
const orchestrion = loadOrchestrionBundlerPlugin();

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 dynamic import for the orchestrion bundler plugin can fail silently, deactivating the useDiagnosticsChannelInjection feature without any warning to the user.
Severity: LOW

Suggested Fix

Add a warning or log message when loadOrchestrionBundlerPlugin() returns undefined inside the webpack configuration. This would alert the user that the useDiagnosticsChannelInjection feature they enabled could not be activated due to a module loading failure.

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/nextjs/src/config/webpack.ts#L433

Potential issue: The orchestrion bundler plugin is loaded dynamically via
`loadModule()`. If this module fails to load for any reason, such as a corrupted
installation, the function returns `undefined`. This causes the
`useDiagnosticsChannelInjection` feature to be silently disabled without any warning or
log being emitted. A user who has explicitly enabled this feature will not be aware that
it is not active, which can hinder debugging and monitoring efforts.

Also affects:

  • packages/nextjs/src/config/turbopack/constructTurbopackConfig.ts:140~140
  • packages/nextjs/src/config/loadOrchestrionBundlerPlugin.ts:28~34

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

if (orchestrion) {
newConfig.plugins.push(orchestrion.sentryOrchestrionWebpackPlugin() as unknown as WebpackPluginInstance);
prependOrchestrionRuntimeExternals(newConfig);
}
}

return newConfig;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import { existsSync } from 'node:fs';
import { isAbsolute } from 'node:path';
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';
import {
BUNDLE_SAFE_INSTRUMENTED_PACKAGES,
externalizeOrchestrionRuntimePackages,
filterInstrumentedExternals,
} from '../../src/config/diagnosticsChannelInjection';

// `externalizeOrchestrionRuntimePackages` loads the orchestrion bundler plugin lazily via a runtime
// `require` that Vitest's module runner can't resolve, so the seam is pointed at the real module
// through `importActual` (which Vitest does resolve).
vi.mock('../../src/config/loadOrchestrionBundlerPlugin', async () => {
const actual = await vi.importActual<Record<string, unknown>>('@sentry/server-utils/orchestrion/webpack');
return { loadOrchestrionBundlerPlugin: () => actual };
});
import { setUpBuildTimeVariables } from '../../src/config/withSentryConfig/buildTime';
import { getServerExternalPackagesPatch } from '../../src/config/withSentryConfig/getFinalConfigObjectBundlerUtils';
import type { NextConfigObject } from '../../src/config/types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ import {
} from '../../../src/config/turbopack/constructTurbopackConfig';
import type { NextConfigObject } from '../../../src/config/types';

// The orchestrion bundler plugin is loaded lazily via `loadOrchestrionBundlerPlugin` (a runtime
// `require` Vitest's module runner can't resolve), so the seam is pointed at the real module
// through `importActual` (which Vitest does resolve).
vi.mock('../../../src/config/loadOrchestrionBundlerPlugin', async () => {
const actual = await vi.importActual<Record<string, unknown>>('@sentry/server-utils/orchestrion/webpack');
return { loadOrchestrionBundlerPlugin: () => actual };
});

// Mock path.resolve to return a predictable loader path
vi.mock('path', async () => {
const actual = await vi.importActual('path');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ import {
} from '../fixtures';
import { materializeFinalNextConfig, materializeFinalWebpackConfig } from '../testUtils';

// Only the plugin factory is stubbed — `resolveOrchestrionRuntimeRequest` must stay real because
// the externals handler under test uses it.
vi.mock('@sentry/server-utils/orchestrion/webpack', async importOriginal => ({
...(await importOriginal<Record<string, unknown>>()),
sentryOrchestrionWebpackPlugin: () => ({ _name: 'sentry-orchestrion-webpack-plugin' }),
}));
// The orchestrion bundler plugin is loaded lazily via `loadOrchestrionBundlerPlugin` (a runtime
// `require` Vitest's module runner can't resolve), so the seam is mocked here rather than the
// subpath. Only the plugin factory is stubbed — `resolveOrchestrionRuntimeRequest` must stay real
// because the externals handler under test uses it.
vi.mock('../../../src/config/loadOrchestrionBundlerPlugin', async () => {
const actual = await vi.importActual<Record<string, unknown>>('@sentry/server-utils/orchestrion/webpack');
return {
loadOrchestrionBundlerPlugin: () => ({
...actual,
sentryOrchestrionWebpackPlugin: () => ({ _name: 'sentry-orchestrion-webpack-plugin' }),
}),
};
});

describe('constructWebpackConfigFunction()', () => {
it('includes expected properties', async () => {
Expand Down
70 changes: 70 additions & 0 deletions packages/nextjs/test/orchestrionStaticImport.test.ts
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*\)`),
);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test loop should use it.each

Low Severity

The second regression case loops over the CJS and ESM builds inside one it, while the sibling case already uses it.each. Per the Testing Conventions review rule, multi-scenario coverage should use it.each / test.each so failures are attributed per build.

Fix in Cursor Fix in Web

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, '\\$&');
}
Loading