From 8cbb9ca00de99004d43e9f2a12e5f36491fd7c99 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 23 Oct 2026 13:48:11 -0700 Subject: [PATCH] test(bundler-tests): Guard server runtime entries against build-time tooling Build tooling keeps leaking into server runtime entries: bundler plugins, orchestrion bundler transforms, Vite, Babel, WASM. Each time, users find it in a different consumer, often in a production deploy (#22794, #23034, #23243, #24775, #25470). Nothing stopped the same shape from coming back. The new test traces each server runtime entry with `@vercel/nft`, the tracer behind Next standalone and the Vercel adapters. It resolves each entry through a fixture, under the export conditions of each runtime, in ESM and CJS. It fails if the trace reaches build-only code, and prints the import chain to it. The trace stops at the first build-time module and does not expand globs. Without that, an entry that does reach build tooling walks all of Vite and Babel, and globs the whole repo, which takes minutes instead of seconds. Every export of a tested package must now be in `CASES`, `BUILD_TIME_CASES`, or `CLIENT_ENTRIES`. This found seven runtime entries that the test did not check, now added to `CASES`. Two of them, `@sentry/remix/v3` and `@sentry/remix/v3/node`, reach the orchestrion load hook. Remix 3 has no bundler, so its asset server runs the transform at request time. They are now known violations. Each build-time entry must reach build-time tooling. If our tooling changes and `BUILD_TIME_PATTERNS` no longer matches it, these cases fail. The test only covers packages it already knows about. A new Bugbot rule flags a new server package, or a new export in an untested one, so that it gets added to the test. Refs #25676 Co-Authored-By: Claude Opus 5.5 (1M context) --- .cursor/BUGBOT.md | 1 + dev-packages/bundler-tests/package.json | 16 + .../tests/runtime-entries.test.ts | 278 ++++++++++++++++++ pnpm-lock.yaml | 43 +++ 4 files changed, 338 insertions(+) create mode 100644 dev-packages/bundler-tests/tests/runtime-entries.test.ts diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index 7091a161f177..4a9d0bd65ef4 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -112,6 +112,7 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - When any `setTimeout` or `setInterval` timers are started in a code path that can end up in server runtime packages (e.g. `@sentry/core` or `@sentry/node`), flag if neither `timeout.unref()` nor `safeUnref()` are called. Not unref'ing a timer can keep CLI-like applications or node scripts from exiting immediately, due to the process waiting on timers started by the SDK. - Flag Node-only imports (`fs`, `path`, `child_process`, `os`, `net`, `http`, `https`, `node:*`, etc.) in code paths that ship to non-Node runtimes (`@sentry/browser`, `@sentry/cloudflare`, `@sentry/deno`, `@sentry/bun`, or shared code in `@sentry/core` / `@sentry/browser-utils` reachable from those entry points). When the dependency is unavoidable, isolate it behind a runtime check, dynamic `import()`, or a Node-only entry point — never at the top level of a cross-runtime module. +- Flag a new package, or a new `exports` entry in a `package.json` under `packages/`, that can load on a server runtime (Node, Bun, Deno, Cloudflare Workers, Vercel Edge) when its package has no entry in `CASES` or `BUILD_TIME_CASES` in `dev-packages/bundler-tests/tests/runtime-entries.test.ts`. That test checks that server runtime entries do not reach build-time tooling (bundler plugins, Vite, Babel, WASM). For packages it already covers, the test fails on its own when an export is not classified, so do not flag those. Recommend adding the package to `devDependencies` in `dev-packages/bundler-tests/package.json`, and each export to `CASES` (runtime), `BUILD_TIME_CASES` (build-time), or `CLIENT_ENTRIES` (browser). Do not flag browser-only packages. ## What NOT to Flag diff --git a/dev-packages/bundler-tests/package.json b/dev-packages/bundler-tests/package.json index 35d63991cbc8..200bc3ea9668 100644 --- a/dev-packages/bundler-tests/package.json +++ b/dev-packages/bundler-tests/package.json @@ -19,5 +19,21 @@ "vitest": "^4.1.11", "webpack": "^5.112.2" }, + "devDependencies": { + "@sentry/astro": "11.10.0", + "@sentry/bun": "11.10.0", + "@sentry/cloudflare": "11.10.0", + "@sentry/deno": "11.10.0", + "@sentry/nextjs": "11.10.0", + "@sentry/node": "11.10.0", + "@sentry/nuxt": "11.10.0", + "@sentry/react-router": "11.10.0", + "@sentry/remix": "11.10.0", + "@sentry/solidstart": "11.10.0", + "@sentry/sveltekit": "11.10.0", + "@sentry/tanstackstart-react": "11.10.0", + "@sentry/vercel-edge": "11.10.0", + "@vercel/nft": "^1.11.0" + }, "type": "module" } diff --git a/dev-packages/bundler-tests/tests/runtime-entries.test.ts b/dev-packages/bundler-tests/tests/runtime-entries.test.ts new file mode 100644 index 000000000000..5b8d881d286d --- /dev/null +++ b/dev-packages/bundler-tests/tests/runtime-entries.test.ts @@ -0,0 +1,278 @@ +import { describe, expect, test } from 'vitest'; +import * as path from 'node:path'; +import * as fs from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +import { nodeFileTrace, resolve } from '@vercel/nft'; + +/** + * Server runtime entries must not reach build-time tooling. When they do, + * the tooling ends up in server bundles and traced outputs (Next standalone, + * Nitro, workerd, ...), where it breaks in a different way for each + * consumer. `@vercel/nft` is the tracer that Next standalone and the Vercel + * adapters use, so it reports what those consumers would ship. + */ + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const ROOT_DIR = path.join(__dirname, '../../..'); +const FIXTURE_DIR = path.join(__dirname, '..', 'dist', 'runtime-entries'); + +const BUILD_TIME_PATTERNS: RegExp[] = [ + /(^|\/)packages\/bundler-plugins\//, + /(^|\/)node_modules\/@sentry\/bundler-plugins(\/|$)/, + /(^|\/)packages\/server-utils\/build\/[^/]+\/orchestrion\/bundler\//, + /(^|\/)node_modules\/(vite|webpack|rollup|esbuild|oxc-parser)\//, + /(^|\/)node_modules\/@(babel|rollup|oxc-parser)\//, + /\.wasm$/, +]; + +const KNOWN_VIOLATIONS = new Set([ + // These root entries also re-export config-time code (`sentryAstro`, + // `withSentry`). They must keep it until v12 for backwards compatibility. + '@sentry/astro', + '@sentry/solidstart', + // Remix 3 has no bundler. Its asset server runs the orchestrion + // transform on browser modules at request time. + '@sentry/remix/v3', + '@sentry/remix/v3/node', +]); + +type Format = 'esm' | 'cjs'; + +interface Case { + specifier: string; + conditions: string[]; + formats: Format[]; +} + +const NODE = ['node']; +const WORKERD = ['workerd', 'worker']; +const BOTH: Format[] = ['esm', 'cjs']; +const ESM: Format[] = ['esm']; + +const CASES: Case[] = [ + { specifier: '@sentry/node', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/node/import', conditions: NODE, formats: ESM }, + { specifier: '@sentry/bun', conditions: ['bun', 'node'], formats: BOTH }, + { specifier: '@sentry/deno', conditions: ['deno', 'node'], formats: ESM }, + { specifier: '@sentry/vercel-edge', conditions: ['edge-light'], formats: BOTH }, + { specifier: '@sentry/cloudflare', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/cloudflare', conditions: [...WORKERD, 'development'], formats: BOTH }, + { specifier: '@sentry/cloudflare/request', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/cloudflare/orchestrion-diagnostics-channel', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/deno/import', conditions: ['deno', 'node'], formats: ESM }, + + { specifier: '@sentry/nextjs', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nextjs', conditions: ['edge-light'], formats: BOTH }, + { specifier: '@sentry/nextjs', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/nextjs/cloudflare', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/nextjs/import', conditions: NODE, formats: ESM }, + { specifier: '@sentry/nextjs/tunnel', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nextjs/tunnel', conditions: ['edge-light'], formats: BOTH }, + { specifier: '@sentry/nextjs/async-storage-shim', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nextjs/orchestrion-runtime/index', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nextjs/orchestrion-runtime/register', conditions: NODE, formats: BOTH }, + + { specifier: '@sentry/nuxt', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nuxt/module/plugins', conditions: NODE, formats: ESM }, + + { specifier: '@sentry/sveltekit', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/sveltekit', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/sveltekit', conditions: ['worker'], formats: BOTH }, + { specifier: '@sentry/sveltekit/opentelemetry-api', conditions: NODE, formats: BOTH }, + + { specifier: '@sentry/astro', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/astro/middleware', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/astro/import', conditions: NODE, formats: ESM }, + + { specifier: '@sentry/react-router', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/react-router', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/react-router/cloudflare', conditions: WORKERD, formats: BOTH }, + + { specifier: '@sentry/remix', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/remix', conditions: ['worker'], formats: BOTH }, + { specifier: '@sentry/remix/cloudflare', conditions: WORKERD, formats: BOTH }, + { specifier: '@sentry/remix/import', conditions: NODE, formats: ESM }, + { specifier: '@sentry/remix/v3', conditions: NODE, formats: ESM }, + { specifier: '@sentry/remix/v3/node', conditions: NODE, formats: ESM }, + + { specifier: '@sentry/solidstart', conditions: NODE, formats: BOTH }, + + { specifier: '@sentry/tanstackstart-react', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/tanstackstart-react/import', conditions: NODE, formats: ESM }, +]; + +/** + * Build-time entries must reach build-time tooling. If one does not, then + * `BUILD_TIME_PATTERNS` no longer matches the tooling that we use. + */ +const BUILD_TIME_CASES: Case[] = [ + { specifier: '@sentry/node/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/node/rollup', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/node/webpack', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/node/esbuild', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/bun/plugin', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/cloudflare/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nextjs/config', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/nuxt/module', conditions: NODE, formats: ESM }, + { specifier: '@sentry/sveltekit/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/react-router/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/remix/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/solidstart/vite', conditions: NODE, formats: BOTH }, + { specifier: '@sentry/tanstackstart-react/vite', conditions: NODE, formats: BOTH }, +]; + +/** Browser entries of the packages above. This test does not check them. */ +const CLIENT_ENTRIES = new Set([ + '@sentry/sveltekit/browser-tracing-variant', + '@sentry/remix/v3/client', + '@sentry/solidstart/solidrouter', +]); + +function writeFixture(specifier: string, conditions: string[], format: Format): string { + fs.mkdirSync(FIXTURE_DIR, { recursive: true }); + const name = `${specifier.replace(/[@/]/g, '_')}-${conditions.join('_')}.${format === 'esm' ? 'mjs' : 'cjs'}`; + const file = path.join(FIXTURE_DIR, name); + fs.writeFileSync(file, format === 'esm' ? `import '${specifier}';\n` : `require('${specifier}');\n`); + return file; +} + +function isBuildTime(file: string): boolean { + return BUILD_TIME_PATTERNS.some(pattern => pattern.test(file)); +} + +type Reasons = Awaited>['reasons']; + +// Most entries share `@sentry/node` and its dependencies, so sharing the +// file and analysis cache saves most of the work. +const traceCache = {}; + +/** The shortest import chain from `entry` to `file`, both relative to `ROOT_DIR`. */ +function importChain(reasons: Reasons, entry: string, file: string): string[] { + const importedBy = new Map([[file, file]]); + const queue = [file]; + while (queue.length) { + const current = queue.shift() as string; + if (current === entry) { + const chain = [current]; + for (let link = current; link !== file;) { + link = importedBy.get(link) as string; + chain.push(link); + } + return chain; + } + for (const parent of reasons.get(current)?.parents ?? []) { + if (!importedBy.has(parent)) { + importedBy.set(parent, current); + queue.push(parent); + } + } + } + return [file]; +} + +/** + * Each place where a runtime module imports a build-time module, as the + * import chain from the entry. + */ +async function findBuildTimeImports(fixture: string, conditions: string[]): Promise { + const imports: [importer: string, file: string][] = []; + + const { fileList, reasons } = await nodeFileTrace([fixture], { + base: ROOT_DIR, + conditions, + cache: traceCache, + // A dynamic `fs` path in config code makes nft glob the whole repo. + // That is slow and finds user files, not package code. + analysis: { emitGlobs: false }, + // Stop at the first build-time module, instead of tracing all of Vite + // or Babel. `.wasm` files are assets, so they stay in the trace with + // their importer. + ignore: file => isBuildTime(file) && !file.endsWith('.wasm'), + async resolve(id, parent, job, cjsResolve) { + const resolved = await resolve(id, parent, job, cjsResolve); + for (const file of [resolved].flat()) { + if (isBuildTime(file)) { + imports.push([path.relative(ROOT_DIR, parent), path.relative(ROOT_DIR, file)]); + } + } + return resolved; + }, + }); + + for (const file of fileList) { + if (file.endsWith('.wasm')) { + for (const parent of reasons.get(file)?.parents ?? []) { + imports.push([parent, file]); + } + } + } + + const entry = path.relative(ROOT_DIR, fixture); + const violations = imports.map(([importer, file]) => + importChain(reasons, entry, importer).concat(file).join('\n -> '), + ); + return [...new Set(violations)].sort(); +} + +describe('server runtime entries do not reach build-time tooling', () => { + for (const { specifier, conditions, formats } of CASES) { + for (const format of formats) { + const name = `${specifier} (${format}, conditions: ${conditions.join(', ')})`; + + if (KNOWN_VIOLATIONS.has(specifier)) { + test(`${name} is a known violation`, async () => { + const violations = await findBuildTimeImports(writeFixture(specifier, conditions, format), conditions); + // Remove the entry from KNOWN_VIOLATIONS when this fails. + expect(violations).not.toEqual([]); + }); + } else { + test(name, async () => { + const violations = await findBuildTimeImports(writeFixture(specifier, conditions, format), conditions); + expect(violations).toEqual([]); + }); + } + } + } +}); + +describe('build-time entries reach build-time tooling', () => { + for (const { specifier, conditions, formats } of BUILD_TIME_CASES) { + for (const format of formats) { + test(`${specifier} (${format}, conditions: ${conditions.join(', ')})`, async () => { + const violations = await findBuildTimeImports(writeFixture(specifier, conditions, format), conditions); + expect(violations).not.toEqual([]); + }); + } + } +}); + +test('every export of a tested package is classified', () => { + const classified = new Set([...CASES, ...BUILD_TIME_CASES].map(({ specifier }) => specifier)); + for (const specifier of CLIENT_ENTRIES) { + classified.add(specifier); + } + const packages = new Set([...classified].map(specifier => specifier.split('/').slice(0, 2).join('/'))); + + const unclassified: string[] = []; + for (const pkg of packages) { + const manifest = path.join(ROOT_DIR, 'packages', pkg.replace('@sentry/', ''), 'package.json'); + const { exports } = JSON.parse(fs.readFileSync(manifest, 'utf8')) as { exports: Record }; + for (const subpath of Object.keys(exports)) { + if (subpath === './package.json') { + continue; + } + const specifier = pkg + subpath.slice(1); + const [prefix, suffix] = specifier.split('*'); + const found = + suffix === undefined + ? classified.has(specifier) + : [...classified].some(s => s.startsWith(prefix as string) && s.endsWith(suffix)); + if (!found) { + unclassified.push(specifier); + } + } + } + // Add each one to CASES, BUILD_TIME_CASES, or CLIENT_ENTRIES. + expect(unclassified).toEqual([]); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e197899faf36..7344e1d1060b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -432,6 +432,49 @@ importers: webpack: specifier: ^5.112.2 version: 5.112.2(clean-css@5.2.2)(cssnano@7.1.9(postcss@8.5.31))(csso@5.0.5)(esbuild@0.28.1)(html-minifier-terser@6.1.0)(lightningcss@1.33.0)(postcss@8.5.31)(sharp@0.35.5(@types/node@20.19.43))(svgo@4.1.0)(uglify-js@3.13.3) + devDependencies: + '@sentry/astro': + specifier: 11.10.0 + version: link:../../packages/astro + '@sentry/bun': + specifier: 11.10.0 + version: link:../../packages/bun + '@sentry/cloudflare': + specifier: 11.10.0 + version: link:../../packages/cloudflare + '@sentry/deno': + specifier: 11.10.0 + version: link:../../packages/deno + '@sentry/nextjs': + specifier: 11.10.0 + version: link:../../packages/nextjs + '@sentry/node': + specifier: 11.10.0 + version: link:../../packages/node + '@sentry/nuxt': + specifier: 11.10.0 + version: link:../../packages/nuxt + '@sentry/react-router': + specifier: 11.10.0 + version: link:../../packages/react-router + '@sentry/remix': + specifier: 11.10.0 + version: link:../../packages/remix + '@sentry/solidstart': + specifier: 11.10.0 + version: link:../../packages/solidstart + '@sentry/sveltekit': + specifier: 11.10.0 + version: link:../../packages/sveltekit + '@sentry/tanstackstart-react': + specifier: 11.10.0 + version: link:../../packages/tanstackstart-react + '@sentry/vercel-edge': + specifier: 11.10.0 + version: link:../../packages/vercel-edge + '@vercel/nft': + specifier: ^1.11.0 + version: 1.11.0(encoding@0.1.13)(rollup@4.62.3)(supports-color@10.0.0) dev-packages/clear-cache-gh-action: dependencies: