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: