From 003f74c84a83d50661959c09efcaeb695c74e4e2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Aug 2026 09:51:31 +0200 Subject: [PATCH 1/3] test(server-utils): Pin orchestrion vendored deps tree-shaking bug Add a regression test that re-bundles the built `orchestrion/register` runtime entry the way a downstream bundler does (honouring the package `sideEffects`) and asserts the vendored CJS deps (meriyah, astring, source-map) stay populated. Under `preserveModules`, `@rollup/plugin-commonjs` emits those deps as empty proxy objects populated by cross-module property writes reachable only through bare side-effect imports; downstream tree-shaking drops those writes, so `parse`/`generate`/the SourceMap constructors become `undefined` and every instrumented module crashes when loaded. The test fails on develop and documents the expected behaviour for the fix. Ref #23664 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/orchestrion/treeshaking.test.ts | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 packages/server-utils/test/orchestrion/treeshaking.test.ts diff --git a/packages/server-utils/test/orchestrion/treeshaking.test.ts b/packages/server-utils/test/orchestrion/treeshaking.test.ts new file mode 100644 index 000000000000..f6a85bb12f15 --- /dev/null +++ b/packages/server-utils/test/orchestrion/treeshaking.test.ts @@ -0,0 +1,90 @@ +import { execSync } from 'node:child_process'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { builtinModules } from 'node:module'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { nodeResolve } from '@rollup/plugin-node-resolve'; +import { rollup } from 'rollup'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +// `@sentry/server-utils/orchestrion/register` (the runtime entry the Node SDK requires from +// `Sentry.init()`) pulls in the vendored orchestrion transformer chain +// (`@apm-js-collab/code-transformer` → meriyah, esquery, astring, source-map). Under +// `preserveModules`, `@rollup/plugin-commonjs` emits each named-export CJS dep as an empty proxy +// object (`var meriyah = {}`) that a *separate* module populates via cross-module property writes +// (`meriyah.parse = parse`), reached only through a bare side-effect import. When a downstream +// bundler (Next.js server, serverless, nitro/vite — rollup and rolldown alike) re-bundles +// `@sentry/node`, its tree-shaker drops those "unused" writes, leaving the proxy empty — so at +// runtime `parse`/`generate`/the SourceMap constructors are `undefined` and every instrumented +// module crashes when loaded. esquery survives only because it ships `module.exports = {…}`. +// +// The fix builds `register`/`hook` without `preserveModules`, so the chain lands in one +// self-contained chunk where each dep's proxy object, its population, and its consumer are +// co-located — and Rollup never drops a property write read within the same module. This test +// re-bundles the built `register` entry exactly the way a downstream bundler does (honouring this +// package's `sideEffects`) and asserts the populations survive. +// See https://github.com/getsentry/sentry-javascript/issues/23664. + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const packageRoot = resolve(__dirname, '../..'); +const registerEntry = join(packageRoot, 'build/esm/orchestrion/runtime/register.js'); + +let tmpDir: string; + +beforeAll(() => { + // The vendored chain only exists after this package's rollup build, so the test operates on + // `build/esm`. CI builds before running unit tests; build on demand for local runs. + if (!existsSync(registerEntry)) { + execSync('yarn build:transpile', { cwd: packageRoot, stdio: 'inherit' }); + } + tmpDir = mkdtempSync(join(tmpdir(), 'orchestrion-treeshake-')); +}); + +afterAll(() => { + if (tmpDir) { + rmSync(tmpDir, { recursive: true, force: true }); + } +}); + +/** + * Re-bundle the built `register` entry the way a downstream bundler would: importing it from this + * package (so Rollup reads its `sideEffects` field) and tree-shaking. Returns the emitted code. + */ +async function reBundleRegisterAsDownstream(): Promise { + const entryPath = join(tmpDir, 'entry.mjs'); + writeFileSync( + entryPath, + [ + `import { registerDiagnosticsChannelInjection } from ${JSON.stringify(registerEntry)};`, + 'export { registerDiagnosticsChannelInjection };', + ].join('\n'), + ); + + const bundle = await rollup({ + input: entryPath, + plugins: [nodeResolve()], + external: id => id === '@sentry/core' || id.startsWith('node:') || builtinModules.includes(id), + onwarn: () => { + /* the vendored graph has benign circular deps; keep the test output quiet */ + }, + }); + + const { output } = await bundle.generate({ format: 'esm' }); + await bundle.close(); + return output[0].code; +} + +describe('vendored orchestrion transformer survives downstream tree-shaking', () => { + it('keeps meriyah, astring and source-map populated after re-bundling the register entry', async () => { + const code = await reBundleRegisterAsDownstream(); + + // Each vendored CJS dep is populated by a cross-module property write. If downstream + // tree-shaking dropped it, the proxy stays `var meriyah = {}` and these assignments vanish — + // the exact breakage from #23664. Their presence means the chain stayed wired up. + expect(code).toContain('meriyah.parse = parse'); + expect(code).toMatch(/astring\.generate =/); + expect(code).toMatch(/sourceMap\.SourceMapConsumer =/); + expect(code).toMatch(/sourceMap\.SourceMapGenerator =/); + }); +}); From 847c4656c3f4ea7f8acc3201cb3b235eee064971 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Aug 2026 10:11:41 +0200 Subject: [PATCH 2/3] fix(server-utils): Bundle orchestrion runtime chain so it survives downstream tree-shaking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `orchestrion/register` and `orchestrion/hook` runtime entrypoints pull in the vendored transformer chain (`@apm-js-collab/code-transformer` → meriyah, esquery, astring, source-map). Under `preserveModules`, `@rollup/plugin-commonjs` emits each named-export CJS dep (meriyah, astring, source-map) as an empty proxy object (`var meriyah = {}`) that a separate module populates via cross-module property writes (`meriyah.parse = parse`), reachable only through a bare side-effect import. When a downstream bundler (Next.js server, serverless, nitro/vite — rollup and rolldown alike) re-bundles `@sentry/node`, its tree-shaker drops those "unused" property writes, leaving the proxy empty. At runtime `parse`/`generate`/the SourceMap constructors are then `undefined`, so any instrumented module throws `TypeError: parse is not a function` when loaded. esquery escaped this only because it ships `module.exports = {…}` (a whole default-export object with nothing to tree-shake off it), which is why the chain was half-shaken rather than fully removed. Build those two entrypoints without `preserveModules` so the whole chain lands in one self-contained shared chunk where each dep's proxy object, its population, and its consumer are co-located in a single module. Rollup never separates a property write from a read within one module, so the chain survives downstream tree-shaking even under this package's `sideEffects: false`. The rest of the package keeps `preserveModules` for fine-grained consumer tree-shaking. Root cause: the bug is in the vendored build's output shape, not package metadata — a `sideEffects` allowlist does not help, because Rollup still applies statement-level dead-code elimination to the cross-module property writes even in a module it considers to have side effects. Co-locating the chain is the only reliable fix. The shared `rollup-plugin-license` instance now runs across both configs; it accumulates scanned deps into one Map, so the last build writes the complete `THIRD-PARTY-LICENSES.txt` union even though the two configs bundle different subsets. Fixes #23664 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/server-utils/rollup.npm.config.mjs | 80 ++++++++++++++++----- 1 file changed, 61 insertions(+), 19 deletions(-) diff --git a/packages/server-utils/rollup.npm.config.mjs b/packages/server-utils/rollup.npm.config.mjs index 6b7c30b5a292..a39257ea6bb2 100644 --- a/packages/server-utils/rollup.npm.config.mjs +++ b/packages/server-utils/rollup.npm.config.mjs @@ -68,8 +68,10 @@ const sanitizedFileNames = info => // notice requirement and the Apache-2.0 §4(d) NOTICE requirement. Only bundled (non-external) // packages are collected — our own `@sentry/*` deps stay external and are excluded. // -// Both the CJS and ESM build variants run this and bundle the same dependency set, so each writes -// the same file; the last write wins and the content is identical. +// This single instance is shared across BOTH the main (`preserveModules`) config and the runtime +// (`register`/`hook`) config below: `rollup-plugin-license` accumulates scanned dependencies into +// one Map across every build it runs in, so whichever build writes last emits the union of both +// configs' bundled deps — a complete list, even though the two configs bundle different subsets. const thirdPartyLicensePlugin = license({ thirdParty: { includePrivate: false, @@ -92,6 +94,50 @@ const orchestrionRuntimeHooks = [ }), ]; +// `interop`/`paths` overrides shared by both configs' outputs (see the main config's inline notes): +// builtins need `'default'` interop and `node:`-prefixed specifiers because the commonjs-converted +// vendored dependencies import them as unprefixed default imports. +const vendorInterop = id => (id && (id.startsWith('node:') || builtinModules.includes(id)) ? 'default' : 'esModule'); +const vendorPaths = Object.fromEntries(builtinModules.map(m => [m, `node:${m}`])); + +// The two runtime entrypoints backing the `./orchestrion/register` and `./orchestrion/hook` subpath +// exports are the only ones that pull in the orchestrion transformer chain +// (`@apm-js-collab/code-transformer` → meriyah/esquery/astring/source-map). Under `preserveModules`, +// `@rollup/plugin-commonjs` emits those CJS deps as an empty proxy object (`var meriyah = {}`) +// populated by a *separate* module through cross-module property writes (`meriyah.parse = parse`) +// reachable only via a bare side-effect import. Downstream re-bundlers (Next.js server, serverless, +// nitro/vite — rollup and rolldown alike) tree-shake those "unused" writes away, leaving the proxy +// empty so `parse`/`generate`/the SourceMap constructors are `undefined` at runtime and every +// instrumented module crashes when loaded (https://github.com/getsentry/sentry-javascript/issues/23664). +// +// So these two entrypoints are built WITHOUT `preserveModules`: the transformer chain lands in one +// self-contained shared chunk where each dep's proxy object, its population, and its consumer are +// co-located in a single module. Rollup never separates a property write from a read within one +// module, so the chain survives downstream tree-shaking even under this package's `sideEffects:false`. +// The rest of the package keeps `preserveModules` (below) for fine-grained consumer tree-shaking. +const orchestrionRuntimeEntrypoints = makeNPMConfigVariants( + makeBaseNPMConfig({ + packageSpecificConfig: { + // Keyed inputs so the entry chunks land at the exact paths the `exports` map points at, even + // without `preserveModules`. + input: { + 'orchestrion/runtime/register': 'src/orchestrion/runtime/register.ts', + 'orchestrion/runtime/hook': 'src/orchestrion/runtime/hook.mjs', + }, + plugins: [debugNodeAlias, commonJSPlugin, thirdPartyLicensePlugin], + output: { + exports: 'named', + preserveModules: false, + entryFileNames: '[name].js', + // The shared transformer chunk sits beside its two entrypoints. + chunkFileNames: 'orchestrion/runtime/vendored-[hash].js', + interop: vendorInterop, + paths: vendorPaths, + }, + }, + }), +); + export default [ ...orchestrionRuntimeHooks, ...makeNPMConfigVariants( @@ -101,21 +147,14 @@ export default [ // `.../orchestrion/vite`, etc.) — none are reachable from `src/index.ts`, so // we list them as separate entrypoints to guarantee they end up in build/esm // and build/cjs. + // `src/orchestrion/runtime/register.ts` (the `./orchestrion/register` subpath the Node SDK + // `require`s from `Sentry.init()`) and `src/orchestrion/runtime/hook.mjs` (the async + // `Module.register()` hooks) are built by the separate `orchestrionRuntimeEntrypoints` config + // above, without `preserveModules` — see the note there. entrypoints: [ 'src/index.ts', 'src/index.no-diagnostic-channels.ts', 'src/orchestrion/config/index.ts', - // `src/orchestrion/runtime/register.ts` backs the `./orchestrion/register` - // subpath export; the Node SDK `require`s it synchronously from - // `Sentry.init()` to install the channel-injection hooks. - 'src/orchestrion/runtime/register.ts', - // The async module hooks passed to `Module.register()`. They load on Node's ESM loader - // thread, which cannot resolve bare specifiers into our bundled dependency graph — but - // relative imports of on-disk files work, and `build/esm` is a `"type": "module"` scope, so - // this entrypoint shares the vendored chunks with the rest of the build. The `./orchestrion/ - // hook` export only maps its `import` condition (nothing ever `require()`s it), so the copy - // in `build/cjs` is unused. - 'src/orchestrion/runtime/hook.mjs', 'src/orchestrion/bundler/vite.ts', 'src/orchestrion/bundler/rollup.ts', 'src/orchestrion/bundler/webpack.ts', @@ -133,14 +172,17 @@ export default [ // The repo default `interop: 'esModule'` dereferences `.default` on default imports of // externals. The commonjs-converted vendored dependencies import Node builtins that way // (e.g. `require('path')` → default import of `path`), and builtins have no `.default` in - // CJS — so builtins need `'default'` interop (the module itself is the default export). - interop: id => (id && (id.startsWith('node:') || builtinModules.includes(id)) ? 'default' : 'esModule'), - // The vendored dependencies import builtins unprefixed (`import … from 'tty'`), which - // Deno rejects outright and vite-node (Node 26) misresolves as a relative path. Emit them - // `node:`-prefixed. - paths: Object.fromEntries(builtinModules.map(m => [m, `node:${m}`])), + // CJS — so builtins need `'default'` interop (the module itself is the default export). The + // vendored deps also import builtins unprefixed (`import … from 'tty'`), which Deno rejects + // and vite-node (Node 26) misresolves as a relative path, so `paths` emits them + // `node:`-prefixed. Both are shared with the runtime config above. + interop: vendorInterop, + paths: vendorPaths, }, }, }), ), + // Built last so its shared `thirdPartyLicensePlugin` instance writes the complete, accumulated + // license list (see the plugin definition above). + ...orchestrionRuntimeEntrypoints, ]; From a78e8549418c5165bd95f2e4d1e7866b836b72e2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 27 Aug 2026 10:28:51 +0200 Subject: [PATCH 3/3] test(server-utils): Run tree-shaking build + re-bundle in beforeAll with a long timeout The nx build cache is Node-version-scoped, so on Node versions other than the one the CI build job ran on, `build/` is absent and the test's on-demand `yarn build:transpile` plus the rollup re-bundle exceeded the default 5s test timeout (the test timed out on Node 22/24/26 while passing on Node 20). Move both the build and the re-bundle into `beforeAll` under a single 180s timeout and reduce the test body to fast string assertions. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../test/orchestrion/treeshaking.test.ts | 45 +++++++++---------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/packages/server-utils/test/orchestrion/treeshaking.test.ts b/packages/server-utils/test/orchestrion/treeshaking.test.ts index f6a85bb12f15..f6751f589494 100644 --- a/packages/server-utils/test/orchestrion/treeshaking.test.ts +++ b/packages/server-utils/test/orchestrion/treeshaking.test.ts @@ -31,27 +31,21 @@ const packageRoot = resolve(__dirname, '../..'); const registerEntry = join(packageRoot, 'build/esm/orchestrion/runtime/register.js'); let tmpDir: string; +let reBundledCode: string; -beforeAll(() => { +// The build + rollup re-bundle happen here, not in the test body, so the whole (potentially slow) +// job runs under one generous timeout. The nx build cache is Node-version-scoped, so on Node +// versions other than the one the CI build job ran on, `build/` is absent and gets built here. +beforeAll(async () => { // The vendored chain only exists after this package's rollup build, so the test operates on - // `build/esm`. CI builds before running unit tests; build on demand for local runs. + // `build/esm`; build on demand when it is missing. if (!existsSync(registerEntry)) { execSync('yarn build:transpile', { cwd: packageRoot, stdio: 'inherit' }); } tmpDir = mkdtempSync(join(tmpdir(), 'orchestrion-treeshake-')); -}); - -afterAll(() => { - if (tmpDir) { - rmSync(tmpDir, { recursive: true, force: true }); - } -}); -/** - * Re-bundle the built `register` entry the way a downstream bundler would: importing it from this - * package (so Rollup reads its `sideEffects` field) and tree-shaking. Returns the emitted code. - */ -async function reBundleRegisterAsDownstream(): Promise { + // Re-bundle the built `register` entry the way a downstream bundler would: importing it from this + // package (so Rollup reads its `sideEffects` field) and tree-shaking. const entryPath = join(tmpDir, 'entry.mjs'); writeFileSync( entryPath, @@ -69,22 +63,25 @@ async function reBundleRegisterAsDownstream(): Promise { /* the vendored graph has benign circular deps; keep the test output quiet */ }, }); - const { output } = await bundle.generate({ format: 'esm' }); await bundle.close(); - return output[0].code; -} + reBundledCode = output[0].code; +}, 180_000); -describe('vendored orchestrion transformer survives downstream tree-shaking', () => { - it('keeps meriyah, astring and source-map populated after re-bundling the register entry', async () => { - const code = await reBundleRegisterAsDownstream(); +afterAll(() => { + if (tmpDir) { + rmSync(tmpDir, { recursive: true, force: true }); + } +}); +describe('vendored orchestrion transformer survives downstream tree-shaking', () => { + it('keeps meriyah, astring and source-map populated after re-bundling the register entry', () => { // Each vendored CJS dep is populated by a cross-module property write. If downstream // tree-shaking dropped it, the proxy stays `var meriyah = {}` and these assignments vanish — // the exact breakage from #23664. Their presence means the chain stayed wired up. - expect(code).toContain('meriyah.parse = parse'); - expect(code).toMatch(/astring\.generate =/); - expect(code).toMatch(/sourceMap\.SourceMapConsumer =/); - expect(code).toMatch(/sourceMap\.SourceMapGenerator =/); + expect(reBundledCode).toContain('meriyah.parse = parse'); + expect(reBundledCode).toMatch(/astring\.generate =/); + expect(reBundledCode).toMatch(/sourceMap\.SourceMapConsumer =/); + expect(reBundledCode).toMatch(/sourceMap\.SourceMapGenerator =/); }); });