Skip to content

Commit 83c644d

Browse files
andreiborzatimfish
andauthored
fix(v10/server-utils): Stop shipping orchestrion bundler plugins as production dependencies (#23667)
Backport of: #22443 Closes: #23611 `@sentry/server-utils` shipped the orchestrion bundler plugins and a full JavaScript parser toolchain (meriyah, esquery, astring, source-map) as production dependencies, so every consumer of `@sentry/node` installed roughly 5 MB of code that no runtime path reaches unless they opt into diagnostics-channel injection. The orchestrion runtime chain is now compiled into this package's own build, and all `@apm-js-collab/*` packages plus meriyah become devDependencies. Production dependencies are down to `@sentry/conventions` and `@sentry/core`. ## Differences to the original PR - `packages/server-utils/src/orchestrion/runtime/register.ts`: kept `RegisterDiagnosticsChannelInjectionOptions` and the optional parameter on `registerDiagnosticsChannelInjection()`, now deprecated and ignored. The original deletes both; on v10 the option is reachable through the public `experimentalUseDiagnosticsChannelInjection(options)`, so removing it would be a compile error for anyone passing `tracingHooksDir`. - `packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts`: same reason, the optional parameter stays. - `packages/server-utils/src/orchestrion/runtime/register.ts`: kept v10's loader-thread guard (`isMainThread` / `parentPort`), which landed after the v10 branch point and is not in the original. - `packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts` and `getFinalConfigObject.ts`: removed `maybeAddOutputFileTracingIncludes` plus its tests in `packages/nextjs/test/config/withSentryConfig.test.ts`. That `@vercel/nft` force-include is a v10-only workaround for meriyah's `module-sync` exports map (#23034), which bundling removes. `develop` never carried it, so this brings both branches to the same state. --------- Co-authored-by: Tim Fish <tim@timfish.uk>
1 parent 04a52dd commit 83c644d

31 files changed

Lines changed: 631 additions & 369 deletions

.size-limit.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,12 +400,12 @@ module.exports = [
400400
import: createImport('init', 'experimentalUseDiagnosticsChannelInjection'),
401401
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
402402
gzip: true,
403-
limit: '154 KB',
403+
limit: '190 KB',
404404
disablePlugins: ['@size-limit/esbuild'],
405405
},
406406
{
407407
name: '@sentry/node/import (ESM hook with diagnostics-channel injection)',
408-
path: ['node_modules/@apm-js-collab/tracing-hooks/hook.mjs', 'packages/node/build/import-hook.mjs'],
408+
path: ['packages/server-utils/build/esm/orchestrion/runtime/hook.js', 'packages/node/build/import-hook.mjs'],
409409
ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
410410
gzip: true,
411411
limit: '76 KB',

dev-packages/e2e-tests/test-applications/nextjs-16-standalone/tests/standalone.test.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,6 @@
1-
import * as fs from 'fs';
2-
import * as path from 'path';
31
import { expect, test } from '@playwright/test';
42
import { waitForError, waitForTransaction } from '@sentry-internal/test-utils';
53

6-
function findFileInDir(dir: string, suffix: string): string | undefined {
7-
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
8-
const entryPath = path.join(dir, entry.name);
9-
if (entry.isDirectory()) {
10-
const found = findFileInDir(entryPath, suffix);
11-
if (found) {
12-
return found;
13-
}
14-
} else if (entryPath.endsWith(suffix)) {
15-
return entryPath;
16-
}
17-
}
18-
return undefined;
19-
}
20-
21-
test('standalone output contains all meriyah files needed at runtime', () => {
22-
// meriyah's ESM build sits behind the `module-sync` export condition, which Next.js' output file
23-
// tracing resolves differently than the Node.js runtime does (https://github.com/vercel/nft/issues/603).
24-
// Without the SDK force-including it, the standalone server crashes with ERR_MODULE_NOT_FOUND.
25-
const standaloneDir = path.join(process.cwd(), '.next', 'standalone');
26-
expect(findFileInDir(standaloneDir, path.join('meriyah', 'dist', 'meriyah.mjs'))).toBeDefined();
27-
expect(findFileInDir(standaloneDir, path.join('meriyah', 'dist', 'meriyah.cjs'))).toBeDefined();
28-
});
29-
304
test('sends a server transaction from the standalone server', async ({ page }) => {
315
const transactionPromise = waitForTransaction('nextjs-16-standalone', transactionEvent => {
326
return transactionEvent.transaction === 'GET /';

dev-packages/e2e-tests/test-applications/nuxt-3-min/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727
},
2828
"pnpm": {
2929
"overrides": {
30-
"ofetch": "1.4.0",
31-
"@vercel/nft": "0.29.4"
30+
"ofetch": "1.4.0"
3231
}
3332
},
3433
"volta": {
Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { resolveOrchestrionRuntimeRequest } from '@sentry/server-utils/orchestrion/webpack';
2+
13
/**
24
* Instrumented packages verified (via e2e) to bundle correctly, removed from Sentry's own
35
* `serverExternalPackages` defaults so the build-time loader can transform them. Everything else
@@ -7,16 +9,55 @@
79
export const BUNDLE_SAFE_INSTRUMENTED_PACKAGES = ['ioredis'];
810

911
/**
10-
* The orchestrion runtime machinery must stay external — its parser breaks when bundled, which
11-
* silently disables the runtime module hook.
12+
* `@sentry/server-utils` (where `register.ts` and the bundled orchestrion runtime ship) must stay
13+
* external: `register.ts` passes its own `__filename`/`import.meta.url` as the `parentURL` for
14+
* `Module.register('@sentry/server-utils/orchestrion/hook.mjs', …)`, so that self-reference only
15+
* resolves while the code still lives at its real `node_modules` location. Bundled into an app
16+
* server chunk instead, the specifier would have to resolve from the chunk's output location,
17+
* which fails under isolated installs (pnpm) where the package is a transitive dependency.
18+
*
19+
* (The `@apm-js-collab/*` packages no longer appear here: they are bundled into
20+
* `@sentry/server-utils`' build, so no import of them exists at runtime.)
1221
*/
13-
export const ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES = [
14-
'@apm-js-collab/tracing-hooks',
15-
'@apm-js-collab/code-transformer',
16-
];
22+
export const ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES = ['@sentry/server-utils'];
1723

1824
/** Remove the given packages from a `serverExternalPackages` list. */
1925
export function filterInstrumentedExternals(externals: string[], packagesToBundle: string[]): string[] {
2026
const set = new Set(packagesToBundle);
2127
return externals.filter(name => !set.has(name));
2228
}
29+
30+
/**
31+
* A webpack `externals` array entry that keeps {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES} truly
32+
* external by resolving each request to an absolute path at build time and emitting a
33+
* `commonjs <absolute path>` external.
34+
*
35+
* Listing the packages in `serverExternalPackages` is not enough: Next.js only externalizes a
36+
* package when its bare specifier also resolves from the project root (`resolveExternal`'s
37+
* base-resolve check in `next/dist/build/handle-externals.js`) — otherwise the
38+
* `require('<bare specifier>')` it emits into the chunk would dangle at runtime, so Next silently
39+
* bundles the package instead. Under isolated installs (pnpm) the package is a transitive
40+
* dependency that never resolves from the project root, so the orchestrion runtime ended up
41+
* compiled into the server chunk — breaking the `Module.register` self-reference described on
42+
* {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES}. Absolute paths sidestep all of this — webpack
43+
* emits `require('/abs/path/…')`, which loads the real files from `node_modules` no matter where
44+
* the chunk lives.
45+
*
46+
* Must be placed *before* Next's own externals handler in the `externals` array: webpack calls
47+
* array entries in order and stops at the first one that returns a result.
48+
*/
49+
export async function externalizeOrchestrionRuntimePackages({
50+
request,
51+
}: {
52+
request?: string;
53+
}): Promise<string | undefined> {
54+
if (
55+
!request ||
56+
!ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES.some(pkg => request === pkg || request.startsWith(`${pkg}/`))
57+
) {
58+
return undefined;
59+
}
60+
61+
const resolved = resolveOrchestrionRuntimeRequest(request);
62+
return resolved ? `commonjs ${resolved}` : undefined;
63+
}

packages/nextjs/src/config/webpack.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as fs from 'fs';
66
import { createRequire } from 'module';
77
import * as path from 'path';
88
import type { VercelCronsConfig } from '../common/types';
9+
import { externalizeOrchestrionRuntimePackages } from './diagnosticsChannelInjection';
910
import { getBuildPluginOptions, normalizePathForGlob } from './getBuildPluginOptions';
1011
import type { RouteManifest } from './manifest/types';
1112
// Note: If you need to import a type from Webpack, do it in `types.ts` and export it from there. Otherwise, our
@@ -434,6 +435,7 @@ export function constructWebpackConfigFunction({
434435
// Orchestrion code-transform loader — Node server runtime only, never the edge compilation
435436
if (runtime === 'server' && userSentryOptions._experimental?.useDiagnosticsChannelInjection) {
436437
newConfig.plugins.push(sentryOrchestrionWebpackPlugin() as unknown as WebpackPluginInstance);
438+
prependOrchestrionRuntimeExternals(newConfig);
437439
}
438440

439441
return newConfig;
@@ -872,6 +874,23 @@ function addOtelWarningIgnoreRule(newConfig: WebpackConfigObjectWithModuleRules)
872874
}
873875
}
874876

877+
/**
878+
* Prepends {@link externalizeOrchestrionRuntimePackages} to `newConfig.externals`, ahead of
879+
* Next.js's own externals handler, so the orchestrion runtime packages stay external even where
880+
* `serverExternalPackages` can't keep them so. See that function's docs for why this is necessary.
881+
*/
882+
function prependOrchestrionRuntimeExternals(newConfig: WebpackConfigObjectWithModuleRules): void {
883+
const existingExternals = newConfig.externals;
884+
885+
if (Array.isArray(existingExternals)) {
886+
existingExternals.unshift(externalizeOrchestrionRuntimePackages);
887+
} else if (existingExternals === undefined) {
888+
newConfig.externals = [externalizeOrchestrionRuntimePackages];
889+
} else {
890+
newConfig.externals = [externalizeOrchestrionRuntimePackages, existingExternals];
891+
}
892+
}
893+
875894
function addEdgeRuntimePolyfills(newConfig: WebpackConfigObjectWithModuleRules, buildContext: BuildContext): void {
876895
// Use ProvidePlugin to inject performance global only when accessed
877896
newConfig.plugins = newConfig.plugins || [];

packages/nextjs/src/config/withSentryConfig/buildTime.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as childProcess from 'child_process';
22
import * as fs from 'fs';
33
import * as path from 'path';
4-
import { getTracingHooksDirectory } from '@sentry/server-utils/orchestrion/webpack';
54
import type { NextConfigObject, SentryBuildOptions } from '../types';
65

76
/**
@@ -54,9 +53,6 @@ export function setUpBuildTimeVariables(
5453
// Marker read by the server SDK to warn if the runtime opt-in call is missing.
5554
if (userSentryOptions._experimental?.useDiagnosticsChannelInjection) {
5655
buildTimeVariables._sentryUseDiagnosticsChannelInjection = 'true';
57-
// Resolved here (where the SDK is a real on-disk package) and inlined, because the runtime
58-
// module hook can't resolve the bare specifier from a bundled server chunk under pnpm.
59-
buildTimeVariables._sentryOrchestrionTracingHooksDir = getTracingHooksDirectory();
6056
}
6157

6258
if (basePath) {

packages/nextjs/src/config/withSentryConfig/getFinalConfigObject.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
} from './getFinalConfigObjectBundlerUtils';
1717
import {
1818
getNextMajor,
19-
maybeAddOutputFileTracingIncludes,
2019
maybeCreateRouteManifest,
2120
maybeGetVercelCronsConfig,
2221
maybeSetClientTraceMetadataOption,
@@ -54,7 +53,6 @@ export function getFinalConfigObject(
5453

5554
maybeSetClientTraceMetadataOption(incomingUserNextConfigObject, nextJsVersion);
5655
maybeSetInstrumentationHookOption(incomingUserNextConfigObject, nextJsVersion);
57-
maybeAddOutputFileTracingIncludes(incomingUserNextConfigObject, nextJsVersion);
5856
warnIfMissingOnRouterTransitionStartHook(userSentryOptions);
5957

6058
const bundlerInfo = getBundlerInfo(nextJsVersion);

packages/nextjs/src/config/withSentryConfig/getFinalConfigObjectUtils.ts

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { debug, isMatchingPattern, parseSemver } from '@sentry/core';
22
import { getSentryRelease } from '@sentry/node';
33
import * as fs from 'fs';
4-
import { createRequire } from 'module';
54
import * as path from 'path';
65
import type { VercelCronsConfig } from '../../common/types';
76
import { createRouteManifest } from '../manifest/createRouteManifest';
@@ -262,72 +261,6 @@ export function getNextMajor(nextJsVersion: string | undefined): number | undefi
262261
return major;
263262
}
264263

265-
/**
266-
* Forces `meriyah`'s runtime files into Next.js' output file tracing.
267-
*
268-
* `meriyah` (a runtime dependency of `@sentry/server-utils`) declares its ESM build behind the
269-
* `module-sync` export condition. Whether `@vercel/nft` traces that condition's target depends on
270-
* the Node.js version running the build and on the nft version the installed Next.js bundles, so
271-
* the traced file set can miss the file Node actually resolves at runtime. `output: 'standalone'`
272-
* builds (and Vercel deployments) then crash on boot with `ERR_MODULE_NOT_FOUND` for
273-
* `meriyah/dist/meriyah.mjs`. Force-including both build outputs makes the trace independent of
274-
* the build environment. (https://github.com/vercel/nft/issues/603,
275-
* https://github.com/getsentry/sentry-javascript/issues/23034)
276-
*
277-
* Only applied on Next.js >= 14.1: before that, `collectBuildTraces` receives
278-
* `Object.entries(pageInfos)` of a `Map` (i.e. an empty array), so its "edge routes have no trace
279-
* files" guard never matches and any include glob matching a pages-router edge route crashes the
280-
* build with ENOENT on the route's missing `.nft.json` (fixed by
281-
* https://github.com/vercel/next.js/pull/59157, released in 14.1.0).
282-
*
283-
* Note: this mutates `incomingUserNextConfigObject`.
284-
*/
285-
export function maybeAddOutputFileTracingIncludes(
286-
incomingUserNextConfigObject: NextConfigObject,
287-
nextJsVersion: string | undefined,
288-
): void {
289-
if (!nextJsVersion) {
290-
return;
291-
}
292-
293-
const { major, minor } = parseSemver(nextJsVersion);
294-
if (major === undefined || minor === undefined || major < 14 || (major === 14 && minor < 1)) {
295-
return;
296-
}
297-
298-
let meriyahDistDir: string;
299-
try {
300-
// Resolve through the dependency chain (@sentry/nextjs -> @sentry/server-utils -> meriyah) so
301-
// the resolved copy is the one the runtime hook actually loads, even with strict pnpm layouts.
302-
const serverUtilsPkgPath = createRequire(`${__dirname}/`).resolve('@sentry/server-utils/package.json');
303-
meriyahDistDir = path.dirname(createRequire(serverUtilsPkgPath).resolve('meriyah'));
304-
} catch {
305-
return;
306-
}
307-
308-
// Include globs are resolved relative to the project directory; normalize to posix separators
309-
// since both the glob matching and Turbopack's native implementation expect forward slashes.
310-
const meriyahIncludes = ['meriyah.mjs', 'meriyah.cjs'].map(file =>
311-
path.relative(process.cwd(), path.join(meriyahDistDir, file)).replace(/\\/g, '/'),
312-
);
313-
314-
const mergeIncludes = (existing: Record<string, string[]> | undefined): Record<string, string[]> => ({
315-
...existing,
316-
'/*': [...new Set([...(existing?.['/*'] ?? []), ...meriyahIncludes])],
317-
});
318-
319-
if (major >= 15) {
320-
incomingUserNextConfigObject.outputFileTracingIncludes = mergeIncludes(
321-
incomingUserNextConfigObject.outputFileTracingIncludes,
322-
);
323-
} else {
324-
incomingUserNextConfigObject.experimental = {
325-
...incomingUserNextConfigObject.experimental,
326-
outputFileTracingIncludes: mergeIncludes(incomingUserNextConfigObject.experimental?.outputFileTracingIncludes),
327-
};
328-
}
329-
}
330-
331264
/**
332265
* Reads the Vercel crons configuration from vercel.json.
333266
* Returns undefined if vercel.json doesn't exist or doesn't contain crons.

packages/nextjs/src/server/index.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,23 +48,15 @@ const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
4848
_sentryRewriteFramesDistDir?: string;
4949
_sentryRelease?: string;
5050
_sentryUseDiagnosticsChannelInjection?: string;
51-
_sentryOrchestrionTracingHooksDir?: string;
5251
};
5352

5453
/**
5554
* EXPERIMENTAL: Next.js-aware variant of `Sentry.experimentalUseDiagnosticsChannelInjection()`
5655
* from `@sentry/node` (see its docs for behavior and caveats).
57-
*
58-
* Next.js bundles the SDK into the server build, from where the runtime module hook can't resolve
59-
* the `@apm-js-collab/tracing-hooks` bare specifier under isolated installs (pnpm). This variant
60-
* points the hook at the package location that `withSentryConfig` resolved at build time.
61-
*
6256
* @experimental May change or be removed in any release.
6357
*/
6458
export function experimentalUseDiagnosticsChannelInjection(): void {
65-
const tracingHooksDir =
66-
process.env._sentryOrchestrionTracingHooksDir || globalWithInjectedValues._sentryOrchestrionTracingHooksDir;
67-
nodeExperimentalUseDiagnosticsChannelInjection(tracingHooksDir ? { tracingHooksDir } : undefined);
59+
nodeExperimentalUseDiagnosticsChannelInjection();
6860
}
6961

7062
// Call at module level so `next build` prerender workers still register the runner without `init`

packages/nextjs/test/config/diagnosticsChannelInjection.test.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import { existsSync } from 'node:fs';
2+
import { isAbsolute } from 'node:path';
13
import { describe, expect, it } from 'vitest';
24
import {
35
BUNDLE_SAFE_INSTRUMENTED_PACKAGES,
6+
externalizeOrchestrionRuntimePackages,
47
filterInstrumentedExternals,
58
} from '../../src/config/diagnosticsChannelInjection';
69
import { setUpBuildTimeVariables } from '../../src/config/withSentryConfig/buildTime';
@@ -33,8 +36,7 @@ describe('getServerExternalPackagesPatch (diagnostics-channel injection)', () =>
3336
expect(externals).toContain('pg');
3437
expect(externals).toContain('pg-pool');
3538
// The orchestrion machinery must be external for the runtime hook to work.
36-
expect(externals).toContain('@apm-js-collab/tracing-hooks');
37-
expect(externals).toContain('@apm-js-collab/code-transformer');
39+
expect(externals).toContain('@sentry/server-utils');
3840
});
3941

4042
it('respects user-provided externals even for bundle-safe packages', () => {
@@ -51,15 +53,50 @@ describe('getServerExternalPackagesPatch (diagnostics-channel injection)', () =>
5153
});
5254
});
5355

56+
describe('externalizeOrchestrionRuntimePackages', () => {
57+
it.each(['@sentry/server-utils', '@sentry/server-utils/orchestrion', '@sentry/server-utils/orchestrion/register'])(
58+
'externalizes %s as an absolute-path commonjs require',
59+
async request => {
60+
const external = await externalizeOrchestrionRuntimePackages({ request });
61+
62+
expect(external).toMatch(/^commonjs /);
63+
const resolvedPath = external!.slice('commonjs '.length);
64+
expect(isAbsolute(resolvedPath)).toBe(true);
65+
expect(existsSync(resolvedPath)).toBe(true);
66+
},
67+
);
68+
69+
it('ignores the bundled @apm-js-collab packages — no import of them exists in the dist anymore', async () => {
70+
await expect(
71+
externalizeOrchestrionRuntimePackages({ request: '@apm-js-collab/tracing-hooks' }),
72+
).resolves.toBeUndefined();
73+
});
74+
75+
it('resolves @sentry/server-utils subpaths to the CJS build, since the emitted external is a require()', async () => {
76+
const external = await externalizeOrchestrionRuntimePackages({
77+
request: '@sentry/server-utils/orchestrion/register',
78+
});
79+
80+
expect(external).toMatch(/[/\\]cjs[/\\]/);
81+
});
82+
83+
it('ignores unrelated requests so later externals handlers still run', async () => {
84+
await expect(externalizeOrchestrionRuntimePackages({ request: 'some-other-package' })).resolves.toBeUndefined();
85+
// Prefix matching must not leak beyond a package-name boundary.
86+
await expect(
87+
externalizeOrchestrionRuntimePackages({ request: '@sentry/server-utils-extras' }),
88+
).resolves.toBeUndefined();
89+
await expect(externalizeOrchestrionRuntimePackages({})).resolves.toBeUndefined();
90+
});
91+
});
92+
5493
describe('setUpBuildTimeVariables (diagnostics-channel injection)', () => {
5594
it('injects the flag marker and the tracing-hooks location', () => {
5695
const nextConfig: NextConfigObject = {};
5796
setUpBuildTimeVariables(nextConfig, { _experimental: { useDiagnosticsChannelInjection: true } }, undefined);
5897

5998
expect(nextConfig.env).toMatchObject({
6099
_sentryUseDiagnosticsChannelInjection: 'true',
61-
// The runtime module hook joins subpaths onto this, so it must be an absolute directory.
62-
_sentryOrchestrionTracingHooksDir: expect.stringMatching(/@apm-js-collab[/+]tracing-hooks/),
63100
});
64101
});
65102

@@ -68,6 +105,5 @@ describe('setUpBuildTimeVariables (diagnostics-channel injection)', () => {
68105
setUpBuildTimeVariables(nextConfig, {}, undefined);
69106

70107
expect(nextConfig.env).not.toHaveProperty('_sentryUseDiagnosticsChannelInjection');
71-
expect(nextConfig.env).not.toHaveProperty('_sentryOrchestrionTracingHooksDir');
72108
});
73109
});

0 commit comments

Comments
 (0)