From 33a972ecbe4109827e2d328d83a8f017ce9a1d1f Mon Sep 17 00:00:00 2001 From: Sigrid <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:26:47 +0200 Subject: [PATCH] fix(v10/server-utils): Bundle the CommonJS dependencies of instrumented packages on Vite 8 Backport of: #25523 Co-Authored-By: GPT-6 --- .../package.json | 11 +- .../tests/server-build.test.ts | 27 +++ .../orchestrion/bundler/externalRequire.ts | 126 ++++++++++++ .../src/orchestrion/bundler/vite.ts | 4 + .../test/orchestrion/externalRequire.test.ts | 183 ++++++++++++++++++ 5 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/tests/server-build.test.ts create mode 100644 packages/server-utils/src/orchestrion/bundler/externalRequire.ts create mode 100644 packages/server-utils/test/orchestrion/externalRequire.test.ts diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/package.json b/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/package.json index 39f2b15a240c..69edefb5eb25 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/package.json +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/package.json @@ -10,7 +10,8 @@ "clean": "npx rimraf node_modules pnpm-lock.yaml", "test:build": "pnpm install && pnpm build", "test:build-latest": "pnpm add @tanstack/react-start@latest @tanstack/react-router@latest && pnpm install && pnpm build", - "test:assert": "pnpm test" + "test:assert": "pnpm test", + "test:build:hoisted": "pnpm install --config.node-linker=hoisted && pnpm build" }, "//": "Need to use ioredis 5.10.1 because that's the last version before they support tracing channels", "dependencies": { @@ -39,5 +40,13 @@ }, "volta": { "extends": "../../package.json" + }, + "sentryTest": { + "variants": [ + { + "label": "tanstackstart-react-orchestrion (hoisted)", + "build-command": "pnpm test:build:hoisted" + } + ] } } diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/tests/server-build.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/tests/server-build.test.ts new file mode 100644 index 000000000000..3aca61976687 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react-orchestrion/tests/server-build.test.ts @@ -0,0 +1,27 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { expect, test } from '@playwright/test'; + +// Nitro can't follow the `__require()` calls that Vite 8 emits for external requires in bundled +// CommonJS, so the deployed server fails with `Cannot find module`. Only flat installs (the hoisted +// variant) keep the drivers' dependencies external. +// https://vite.dev/guide/migration#require-calls-for-externalized-modules +test('bundles the CommonJS dependencies of the instrumented drivers', () => { + const root = path.join(process.cwd(), '.output/server'); + const serverBundle = readdirSync(root, { recursive: true, encoding: 'utf8' }) + .filter(file => /\.m?js$/.test(file) && !file.startsWith('node_modules')) + .map(file => readFileSync(path.join(root, file), 'utf8')) + .join('\n'); + const driverDependencies = ['sqlstring', 'bignumber.js', 'denque', 'cluster-key-slot', 'standard-as-callback']; + + const externalRequires = [...serverBundle.matchAll(/__require\(["']([^"']+)["']\)/g)].map(match => match[1]); + + const imports = [ + ...serverBundle.matchAll(/\b(?:__require|require|import)\(\s*["']([^"']+)["']\s*\)|\bfrom\s*["']([^"']+)["']/g), + ].map(match => match[1] ?? match[2]); + expect( + imports.filter(name => name === 'mysql'), + 'mysql must be bundled to be instrumented', + ).toEqual([]); + expect(externalRequires.filter(name => driverDependencies.includes(name))).toEqual([]); +}); diff --git a/packages/server-utils/src/orchestrion/bundler/externalRequire.ts b/packages/server-utils/src/orchestrion/bundler/externalRequire.ts new file mode 100644 index 000000000000..7eea2472739a --- /dev/null +++ b/packages/server-utils/src/orchestrion/bundler/externalRequire.ts @@ -0,0 +1,126 @@ +import { isPlainObject } from '@sentry/core'; +import { readFileSync } from 'node:fs'; +import { createRequire, isBuiltin } from 'node:module'; +import { isAbsolute } from 'node:path'; +import type { Plugin } from 'vite'; + +type ResolveIdHandler = Extract, (...args: never[]) => unknown>; +type ResolveIdParameters = Parameters; + +const NODE_MODULES = '/node_modules/'; + +/** + * Creates a `resolveId` handler that bundles the external packages that bundled CommonJS `require()`s. + * + * Rolldown keeps these calls as `__require()`, which a second bundler over the server output can't follow: + * - https://vite.dev/guide/migration#require-calls-for-externalized-modules + * - https://rolldown.rs/in-depth/bundling-cjs#require-external-modules + * + * Only handles requires from packages bundled for orchestrion, and from packages bundled because of them. + */ +export function createExternalRequireResolver( + moduleNames: readonly string[], +): (this: ThisParameterType, ...args: ResolveIdParameters) => Promise { + const bundledPackagesByEnvironment = new Map>(); + + return async function resolveExternalRequire(source, importer, options) { + if ( + !(this.meta as typeof this.meta & { rolldownVersion?: string }).rolldownVersion || + this.environment.mode !== 'build' || + (options as typeof options & { kind?: string }).kind !== 'require-call' || + !importer || + !isPackageSpecifier(source) || + source.startsWith('@sentry/') + ) { + return null; + } + + const bundledPackages = bundledPackagesByEnvironment.get(this.environment.name) ?? new Set(moduleNames); + bundledPackagesByEnvironment.set(this.environment.name, bundledPackages); + + const owner = ownerPackage(importer); + const name = packageName(source); + if ( + !owner || + !bundledPackages.has(owner.name) || + isUserExternal(this.environment.config.resolve.external, source, name) || + isOptionalDependency(owner.dir, name) + ) { + return null; + } + + const resolved = await this.resolve(source, importer, { ...options, skipSelf: true }); + if (!resolved) { + return null; + } + // Vite bundles it anyway (e.g. under pnpm), so its own requires count too. + if (!resolved.external) { + bundledPackages.add(name); + return null; + } + + const file = resolveWithNode(source, importer); + if (file) { + bundledPackages.add(name); + } + return file ?? null; + }; +} + +function isPackageSpecifier(source: string): boolean { + return !isBuiltin(source) && !isAbsolute(source) && !/^[.\0]/.test(source); +} + +// `pg-types/lib/x` → `pg-types`, `@scope/pkg/x` → `@scope/pkg`. +function packageName(specifier: string): string { + const [first = '', second] = specifier.split('/'); + return first.startsWith('@') && second ? `${first}/${second}` : first; +} + +// Uses the last `node_modules` segment, so nested and pnpm store paths work. +function ownerPackage(file: string): { name: string; dir: string } | undefined { + const path = file.replace(/\\/g, '/'); + const start = path.lastIndexOf(NODE_MODULES); + if (start === -1) { + return undefined; + } + const name = packageName(path.slice(start + NODE_MODULES.length)); + return { name, dir: `${path.slice(0, start)}${NODE_MODULES}${name}` }; +} + +// Matches like Vite. `external: true` is Vite's SSR default, not a choice for this package. +function isUserExternal(external: readonly string[] | true, source: string, name: string): boolean { + return Array.isArray(external) && external.some(entry => entry === source || entry === name); +} + +// These can be missing or native, like `pg-native`. +function isOptionalDependency(packageDir: string, name: string): boolean { + const manifest = readManifest(`${packageDir}/package.json`); + if (!manifest) { + return false; + } + const { peerDependenciesMeta, optionalDependencies } = manifest; + const peerMeta = isPlainObject(peerDependenciesMeta) ? peerDependenciesMeta[name] : undefined; + return ( + (isPlainObject(peerMeta) && peerMeta.optional === true) || + (isPlainObject(optionalDependencies) && name in optionalDependencies) + ); +} + +// The file that `require()` loads at runtime. +function resolveWithNode(source: string, importer: string): string | undefined { + try { + return createRequire(importer).resolve(source); + } catch { + return undefined; + } +} + +function readManifest(file: string): Record | undefined { + try { + const manifest: unknown = JSON.parse(readFileSync(file, 'utf8')); + return isPlainObject(manifest) ? manifest : undefined; + } catch { + return undefined; + } +} diff --git a/packages/server-utils/src/orchestrion/bundler/vite.ts b/packages/server-utils/src/orchestrion/bundler/vite.ts index dc3426e9ba02..381d2a62c997 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -1,3 +1,4 @@ +import { createExternalRequireResolver } from './externalRequire'; import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/vite'; import type { Plugin, ResolvedConfig } from 'vite'; import { instrumentedModuleNames } from '../config'; @@ -19,8 +20,11 @@ import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTran * ``` */ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { + const resolveExternalRequire = createExternalRequireResolver(instrumentedModuleNames(options.instrumentations)); + return { ...codeTransformer(orchestrionTransformOptions(options)), + resolveId: resolveExternalRequire, applyToEnvironment(environment) { // Orchestrion splices `node:diagnostics_channel` calls into instrumented modules, which only // exist server-side. Only apply to server-consumed environments so injected `tracingChannel` diff --git a/packages/server-utils/test/orchestrion/externalRequire.test.ts b/packages/server-utils/test/orchestrion/externalRequire.test.ts new file mode 100644 index 000000000000..60ee79a14fd6 --- /dev/null +++ b/packages/server-utils/test/orchestrion/externalRequire.test.ts @@ -0,0 +1,183 @@ +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createExternalRequireResolver } from '../../src/orchestrion/bundler/externalRequire'; + +interface ContextOptions { + environment?: string; + meta?: Record; + mode?: string; + external?: string[] | true; + resolved?: { id: string; external?: boolean } | null; +} + +describe('createExternalRequireResolver', () => { + let root: string; + let pgFile: string; + + function installPackage(name: string, manifest: Record = {}, parentDir = root): string { + const dir = join(parentDir, 'node_modules', name); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'package.json'), JSON.stringify({ name, main: 'index.js', ...manifest })); + writeFileSync(join(dir, 'index.js'), ''); + return join(dir, 'index.js'); + } + + // By default, a Rolldown build in which Vite keeps the required package external. + function context({ + environment = 'ssr', + meta = { rolldownVersion: '1.2.14' }, + mode = 'build', + external = [], + resolved = { id: 'external', external: true }, + }: ContextOptions = {}): never { + return { + meta, + environment: { name: environment, mode, config: { resolve: { external } } }, + resolve: vi.fn().mockResolvedValue(resolved), + } as never; + } + + function resolveRequire( + resolver: ReturnType, + source: string, + importer: string, + contextOptions?: ContextOptions, + kind = 'require-call', + ): Promise { + return resolver.call(context(contextOptions), source, importer, { kind, isEntry: false } as never); + } + + beforeEach(() => { + root = realpathSync(mkdtempSync(join(tmpdir(), 'orch-external-require-'))); + pgFile = join(root, 'node_modules/pg/lib/client.js'); + installPackage('pg'); + }); + + afterEach(() => { + rmSync(root, { recursive: true, force: true }); + }); + + it('bundles an external package that an instrumented package requires', async () => { + const pgTypes = installPackage('pg-types'); + + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile)).resolves.toBe(pgTypes); + }); + + it.each(['@sentry/core', '@sentry/server-utils', '@sentry/server-utils/orchestrion'])( + 'preserves the existing resolution of the SDK import %s', + async source => { + installPackage('@sentry/core'); + installPackage('@sentry/server-utils', { exports: { '.': './index.js', './orchestrion': './index.js' } }); + + await expect(resolveRequire(createExternalRequireResolver(['pg']), source, pgFile)).resolves.toBeNull(); + }, + ); + + it.each([ + ['it bundled', { id: 'external', external: true }], + ['Vite bundles itself', { id: 'pg-types/index.js' }], + ])('follows the requires of a package that %s', async (_, resolved) => { + const pgTypes = installPackage('pg-types'); + const postgresArray = installPackage('postgres-array'); + const resolver = createExternalRequireResolver(['pg']); + + await resolveRequire(resolver, 'pg-types', pgFile, { resolved }); + + await expect(resolveRequire(resolver, 'postgres-array', pgTypes)).resolves.toBe(postgresArray); + }); + + // Every require() here resolves to a real file, so only the filter can return `null`. + it.each([ + ['app code', 'pg-types', 'src/db.js'], + ['a package that orchestrion does not bundle', 'pg-types', 'node_modules/other/index.js'], + ['a builtin', 'node:net', 'node_modules/pg/lib/client.js'], + ['a relative path', './connection', 'node_modules/pg/lib/client.js'], + ])('ignores a require() of %s', async (_, source, importer) => { + installPackage('pg-types'); + installPackage('other'); + mkdirSync(join(root, 'node_modules/pg/lib'), { recursive: true }); + writeFileSync(join(root, 'node_modules/pg/lib/connection.js'), ''); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), source, join(root, importer)), + ).resolves.toBeNull(); + }); + + it('bundles a require() from a pnpm store path', async () => { + const store = join(root, 'node_modules/.pnpm/pg@8.23.1'); + installPackage('pg', {}, store); + const pgTypes = installPackage('pg-types', {}, store); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', join(store, 'node_modules/pg/lib/client.js')), + ).resolves.toBe(pgTypes); + }); + + it('tracks bundled packages for each environment separately', async () => { + const pgTypes = installPackage('pg-types'); + installPackage('postgres-array'); + const resolver = createExternalRequireResolver(['pg']); + + await resolveRequire(resolver, 'pg-types', pgFile, { environment: 'ssr' }); + + await expect(resolveRequire(resolver, 'postgres-array', pgTypes, { environment: 'nitro' })).resolves.toBeNull(); + }); + + it.each([ + ['on Rollup', { meta: { rollupVersion: '4.0.0' } }, 'require-call'], + ['on the dev server', { mode: 'dev' }, 'require-call'], + ['for an import', {}, 'import-statement'], + ])('leaves resolution to the bundler %s', async (_, contextOptions, kind) => { + installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, contextOptions, kind), + ).resolves.toBeNull(); + }); + + it.each([ + ['an optional peer', { peerDependenciesMeta: { 'pg-native': { optional: true } } }], + ['an optional dependency', { optionalDependencies: { 'pg-native': '^3' } }], + ])('keeps %s external', async (_, manifest) => { + installPackage('pg', manifest); + installPackage('pg-native'); + + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-native', pgFile)).resolves.toBeNull(); + }); + + it('keeps a scoped subpath external when the user lists its package', async () => { + installPackage('@scope/pkg'); + writeFileSync(join(root, 'node_modules/@scope/pkg/sub.js'), ''); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), '@scope/pkg/sub', pgFile, { external: ['@scope/pkg'] }), + ).resolves.toBeNull(); + }); + + it('keeps a package external that the user lists in `external`', async () => { + installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, { external: ['pg-types'] }), + ).resolves.toBeNull(); + }); + + it.each([ + ['only a subpath of the package', ['pg-types/lib/arrays']], + ['`true`, the SSR default', true], + ] as const)('bundles a package despite an external entry that names %s', async (_, external) => { + const pgTypes = installPackage('pg-types'); + + await expect( + resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile, { + external: external === true ? true : [...external], + }), + ).resolves.toBe(pgTypes); + }); + + it('keeps a package external that is not installed', async () => { + await expect(resolveRequire(createExternalRequireResolver(['pg']), 'pg-types', pgFile)).resolves.toBeNull(); + }); +});