diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json b/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json index d080dc4e25fd..ed739895d0d9 100644 --- a/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/package.json @@ -14,6 +14,7 @@ "test:build:tunnel-static": "pnpm install && E2E_TEST_TUNNEL_ROUTE_MODE=static E2E_TEST_DSN=http://public@localhost:3031/1337 pnpm build", "test:build:tunnel-custom": "pnpm install && E2E_TEST_CUSTOM_TUNNEL_ROUTE=1 pnpm build", "test:build:tunnel-object": "pnpm install && E2E_TEST_TUNNEL_ROUTE_MODE=object E2E_TEST_DSN=http://public@localhost:3031/1337 pnpm build", + "test:build:hoisted": "pnpm install --config.node-linker=hoisted && pnpm build", "test:build-latest": "pnpm add @tanstack/react-start@latest @tanstack/react-router@latest nitro@latest vite@latest @vitejs/plugin-react-swc@latest vite-tsconfig-paths@latest && pnpm install && pnpm build", "test:assert:proxy": "pnpm test && TEST_ENV=development pnpm playwright test db-drivers", "test:assert": "pnpm test:assert:proxy", @@ -51,6 +52,10 @@ }, "sentryTest": { "variants": [ + { + "label": "tanstackstart-react (hoisted)", + "build-command": "pnpm test:build:hoisted" + }, { "label": "tanstackstart-react (tunnel-generated)", "build-command": "pnpm test:build:tunnel-generated", diff --git a/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts new file mode 100644 index 000000000000..57bf54397164 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/tanstackstart-react/tests/server-build.test.ts @@ -0,0 +1,22 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { expect, test } from '@playwright/test'; +import { bundleReferencesModule } from '@sentry-internal/test-utils'; + +// 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]); + + expect(bundleReferencesModule(serverBundle, 'mysql'), 'mysql must be bundled to be instrumented').toBe(false); + 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..971f19c322d6 --- /dev/null +++ b/packages/server-utils/src/orchestrion/bundler/externalRequire.ts @@ -0,0 +1,125 @@ +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.rolldownVersion || + this.environment.mode !== 'build' || + options.kind !== 'require-call' || + !importer || + !isPackageSpecifier(source) + ) { + 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 c6a704bb1ca9..16e642dc76fd 100644 --- a/packages/server-utils/src/orchestrion/bundler/vite.ts +++ b/packages/server-utils/src/orchestrion/bundler/vite.ts @@ -5,6 +5,7 @@ export type { Plugin as VitePlugin } from 'vite'; import { instrumentedModuleNames } from '../config'; import type { PluginOptions } from './options'; import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; +import { createExternalRequireResolver } from './externalRequire'; import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './resolve'; type TransformHandler = (this: unknown, code: string, id: string, opts?: { ssr?: boolean }) => unknown; @@ -73,6 +74,7 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { ...instrumentedModuleNames(options.instrumentations), '@sentry/server-utils', ]; + const resolveExternalRequire = createExternalRequireResolver(noExternalModules()); const instance = {}; // `true` when an earlier instance of this plugin is in the same build, which then transforms the modules alone. @@ -94,7 +96,10 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { // packages into default imports, and `@sentry/core` has no default export, so the server // crashes at startup. Vite 8 keeps the `require()`s, which loads a second copy of `@sentry/core`. async resolveId(source, importer, resolveOptions) { - if (source !== SNIPPET_IMPORT_SPECIFIER || !resolveOptions?.ssr) { + if (source !== SNIPPET_IMPORT_SPECIFIER) { + return resolveExternalRequire.call(this, source, importer, resolveOptions); + } + if (!resolveOptions?.ssr) { return null; } const resolved = await this.resolve(source, importer, { ...resolveOptions, skipSelf: true }); diff --git a/packages/server-utils/test/orchestrion/bundler.test.ts b/packages/server-utils/test/orchestrion/bundler.test.ts index d6cf125df97d..b0690ab74903 100644 --- a/packages/server-utils/test/orchestrion/bundler.test.ts +++ b/packages/server-utils/test/orchestrion/bundler.test.ts @@ -446,6 +446,27 @@ describe('sentryOrchestrionPlugin (vite)', () => { expect(fallback).toMatch(/[/\\]build[/\\]esm[/\\]index\.js$/); expect(existsSync(fallback as string)).toBe(true); }); + + it('resolves the snippet require() to the ESM entry before it handles other requires on Rolldown', async () => { + const resolveId = vitePlugin().resolveId as ( + this: unknown, + source: string, + importer: string, + opts: { ssr: boolean; kind: string }, + ) => Promise; + const context = { + meta: { rolldownVersion: '1.2.14' }, + environment: { name: 'ssr', mode: 'build', config: { resolve: { external: [] } } }, + resolve: vi.fn().mockResolvedValue({ id: '/app/node_modules/@sentry/server-utils/build/cjs/index.js' }), + }; + + const resolved = await resolveId.call(context, '@sentry/server-utils', '/app/node_modules/pg/lib/client.js', { + ssr: true, + kind: 'require-call', + }); + + expect(resolved).toEqual({ id: '/app/node_modules/@sentry/server-utils/build/esm/index.js' }); + }); }); describe('sentryOrchestrionPlugin (bun)', () => { 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..73951a3b971e --- /dev/null +++ b/packages/server-utils/test/orchestrion/externalRequire.test.ts @@ -0,0 +1,173 @@ +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([ + ['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(); + }); +});