diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16-orchestrion/tests/build-output.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-16-orchestrion/tests/build-output.test.ts new file mode 100644 index 000000000000..735e04649c32 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/nextjs-16-orchestrion/tests/build-output.test.ts @@ -0,0 +1,9 @@ +import { expect, test } from '@playwright/test'; +import { findAbsolutePathImports } from '@sentry-internal/test-utils'; +import * as path from 'path'; + +test('emits no absolute-path imports into the server output', () => { + const leaks = findAbsolutePathImports({ outputDir: path.join(process.cwd(), '.next', 'server') }); + + expect(leaks).toEqual([]); +}); diff --git a/dev-packages/test-utils/src/build-output.ts b/dev-packages/test-utils/src/build-output.ts new file mode 100644 index 000000000000..e3aebc97b41d --- /dev/null +++ b/dev-packages/test-utils/src/build-output.ts @@ -0,0 +1,60 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +interface AbsolutePathImportOptions { + /** Directory holding the emitted bundles, e.g. `/.next/server`. */ + outputDir: string; + /** Only used to shorten the reported file paths. Defaults to `process.cwd()`. */ + buildDir?: string; + /** File extensions to scan. Defaults to JavaScript output. */ + extensions?: string[]; +} + +const SPECIFIER_PATTERNS = [ + /\brequire\(\s*["']([^"']+)["']\s*\)/g, + /\bimport\(\s*["']([^"']+)["']\s*\)/g, + /\bfrom\s*["']([^"']+)["']/g, +]; + +/** + * Returns every absolute-path module specifier in the emitted output, as ` → `. + * + * Such a specifier is baked in at build time, so it only resolves on the build machine: every + * deploy that relocates the output (Vercel, Docker, `output: 'standalone'`) turns it into a + * `MODULE_NOT_FOUND` on the first request reaching that chunk. A suite that builds and runs in + * place can't observe that, hence the direct assertion. + * + * Only specifiers count, not any occurrence of a build path — Next.js bakes those into chunks as + * metadata (`resolvedPagePath`, client-reference proxies), which is inert on relocation. + */ +export function findAbsolutePathImports({ + outputDir, + buildDir = process.cwd(), + extensions = ['.js', '.mjs', '.cjs'], +}: AbsolutePathImportOptions): string[] { + if (!fs.existsSync(outputDir)) { + throw new Error(`[findAbsolutePathImports] Output directory does not exist: ${outputDir}`); + } + + const leaks: string[] = []; + + for (const entry of fs.readdirSync(outputDir, { recursive: true, withFileTypes: true })) { + if (!entry.isFile() || !extensions.includes(path.extname(entry.name))) { + continue; + } + + const file = path.join(entry.parentPath, entry.name); + const contents = fs.readFileSync(file, 'utf8'); + + for (const pattern of SPECIFIER_PATTERNS) { + for (const match of contents.matchAll(pattern)) { + const specifier = match[1] as string; + if (path.isAbsolute(specifier)) { + leaks.push(`${path.relative(buildDir, file)} → ${specifier}`); + } + } + } + } + + return leaks; +} diff --git a/dev-packages/test-utils/src/index.ts b/dev-packages/test-utils/src/index.ts index d242a7615d3c..94c16e744211 100644 --- a/dev-packages/test-utils/src/index.ts +++ b/dev-packages/test-utils/src/index.ts @@ -15,6 +15,8 @@ export { getSpanOp, } from './event-proxy-server'; +export { findAbsolutePathImports } from './build-output'; + export { getPlaywrightConfig } from './playwright-config'; export { createBasicSentryServer, createTestServer } from './server'; diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 02c0d9288f70..bf1e7978c213 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -58,6 +58,10 @@ "import": { "default": "./build/import-hook.mjs" } + }, + "./orchestrion-runtime/*": { + "require": "./build/orchestrion-runtime/*.js", + "default": "./build/orchestrion-runtime/*.js" } }, "publishConfig": { @@ -111,5 +115,19 @@ "volta": { "extends": "../../package.json" }, + "nx": { + "targets": { + "build:transpile": { + "outputs": [ + "{projectRoot}/build/esm", + "{projectRoot}/build/cjs", + "{projectRoot}/build/npm/esm", + "{projectRoot}/build/npm/cjs", + "{projectRoot}/build/import-hook.mjs", + "{projectRoot}/build/orchestrion-runtime" + ] + } + } + }, "sideEffects": false } diff --git a/packages/nextjs/scripts/buildRollup.ts b/packages/nextjs/scripts/buildRollup.ts index d273146b872d..69ba523ecb23 100644 --- a/packages/nextjs/scripts/buildRollup.ts +++ b/packages/nextjs/scripts/buildRollup.ts @@ -22,3 +22,30 @@ const esmTemplateDir = 'build/esm/config/templates/'; fs.readdirSync(esmTemplateDir).forEach(templateFile => fs.copyFileSync(path.join(esmTemplateDir, templateFile), path.join(cjsTemplateDir, templateFile)), ); + +// Generate the orchestrion runtime forwarders (see `src/config/diagnosticsChannelInjection.ts`) +// from `@sentry/server-utils`' own exports map, so a new subpath there is forwarded automatically. +// Only `require`-able entries get one, since the emitted external is a `require()`. Written as +// plain CJS, not built by rollup: they are loaded by specifier, never bundled. +const SERVER_UTILS = '@sentry/server-utils'; +const orchestrionRuntimeBuildDir = 'build/orchestrion-runtime'; + +const serverUtilsExports = ( + JSON.parse(fs.readFileSync(require.resolve(`${SERVER_UTILS}/package.json`), 'utf8')) as { + exports: Record; + } +).exports; + +for (const [key, conditions] of Object.entries(serverUtilsExports)) { + if (key === './package.json' || typeof conditions === 'string' || !conditions.require) { + continue; + } + + // '.' → 'index', './orchestrion/register' → 'orchestrion/register' + const forwarderPath = path.join(orchestrionRuntimeBuildDir, `${key === '.' ? 'index' : key.slice(2)}.js`); + fs.mkdirSync(path.dirname(forwarderPath), { recursive: true }); + fs.writeFileSync( + forwarderPath, + `// Generated by scripts/buildRollup.ts — do not edit.\nmodule.exports = require('${SERVER_UTILS}${key.slice(1)}');\n`, + ); +} diff --git a/packages/nextjs/src/config/diagnosticsChannelInjection.ts b/packages/nextjs/src/config/diagnosticsChannelInjection.ts index 74233c87c923..3040fc6041dc 100644 --- a/packages/nextjs/src/config/diagnosticsChannelInjection.ts +++ b/packages/nextjs/src/config/diagnosticsChannelInjection.ts @@ -28,36 +28,48 @@ export function filterInstrumentedExternals(externals: string[], packagesToBundl } /** - * A webpack `externals` array entry that keeps {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES} truly - * external by resolving each request to an absolute path at build time and emitting a - * `commonjs ` external. + * Where the generated forwarders live — one CJS one-liner per `@sentry/server-utils` entrypoint + * (see `scripts/buildRollup.ts`). Forwarding through `@sentry/nextjs`, always a direct dependency, + * is what makes the emitted specifier both resolvable from `.next/server/**` and relocation-safe. + */ +const ORCHESTRION_FORWARDER_PREFIX = '@sentry/nextjs/orchestrion-runtime/'; + +/** The forwarder specifier mirroring `request`: `/a/b` → `…/orchestrion-runtime/a/b`. */ +export function getOrchestrionForwarderSpecifier(request: string, externalPackage: string): string { + const subpath = request.slice(externalPackage.length + 1); + return `${ORCHESTRION_FORWARDER_PREFIX}${subpath || 'index'}`; +} + +/** + * A webpack `externals` array entry that keeps {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES} + * external, via the matching forwarder under {@link ORCHESTRION_FORWARDER_PREFIX}. * - * Listing the packages in `serverExternalPackages` is not enough: Next.js only externalizes a - * package when its bare specifier also resolves from the project root (`resolveExternal`'s - * base-resolve check in `next/dist/build/handle-externals.js`) — otherwise the - * `require('')` it emits into the chunk would dangle at runtime, so Next silently - * bundles the package instead. Under isolated installs (pnpm) the package is a transitive - * dependency that never resolves from the project root, so the orchestrion runtime ended up - * compiled into the server chunk — breaking the `Module.register` self-reference described on - * {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES}. Absolute paths sidestep all of this — webpack - * emits `require('/abs/path/…')`, which loads the real files from `node_modules` no matter where - * the chunk lives. + * `serverExternalPackages` can't do this: Next only externalizes a package whose bare specifier + * also resolves from the project root (`resolveExternal`'s base-resolve check in + * `next/dist/build/handle-externals.js`), and under isolated installs this one doesn't — so Next + * silently bundles it, breaking the `Module.register` self-reference described on + * {@link ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES}. * - * Must be placed *before* Next's own externals handler in the `externals` array: webpack calls - * array entries in order and stops at the first one that returns a result. + * Must be placed *before* Next's own externals handler: webpack calls array entries in order and + * stops at the first result. */ export async function externalizeOrchestrionRuntimePackages({ request, }: { request?: string; }): Promise { - if ( - !request || - !ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES.some(pkg => request === pkg || request.startsWith(`${pkg}/`)) - ) { + const externalPackage = request + ? ORCHESTRION_RUNTIME_EXTERNAL_PACKAGES.find(pkg => request === pkg || request.startsWith(`${pkg}/`)) + : undefined; + + if (!request || !externalPackage) { + return undefined; + } + + // Not `require`-able (ESM-only subpath, or a typo): webpack reports it better than we can. + if (!resolveOrchestrionRuntimeRequest(request)) { return undefined; } - const resolved = resolveOrchestrionRuntimeRequest(request); - return resolved ? `commonjs ${resolved}` : undefined; + return `commonjs ${getOrchestrionForwarderSpecifier(request, externalPackage)}`; } diff --git a/packages/nextjs/test/config/diagnosticsChannelInjection.test.ts b/packages/nextjs/test/config/diagnosticsChannelInjection.test.ts index 234b86b5d0ea..d62db8471974 100644 --- a/packages/nextjs/test/config/diagnosticsChannelInjection.test.ts +++ b/packages/nextjs/test/config/diagnosticsChannelInjection.test.ts @@ -1,10 +1,13 @@ import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { fileURLToPath } from 'node:url'; import { isAbsolute } from 'node:path'; import { describe, expect, it } from 'vitest'; import { BUNDLE_SAFE_INSTRUMENTED_PACKAGES, externalizeOrchestrionRuntimePackages, filterInstrumentedExternals, + getOrchestrionForwarderSpecifier, } from '../../src/config/diagnosticsChannelInjection'; import type { BundlerInfo } from '../../src/config/withSentryConfig/getFinalConfigObjectBundlerUtils'; import { @@ -56,17 +59,18 @@ describe('getServerExternalPackagesPatch (build-time instrumentation)', () => { }); describe('externalizeOrchestrionRuntimePackages', () => { - it.each(['@sentry/server-utils', '@sentry/server-utils/orchestrion', '@sentry/server-utils/orchestrion/register'])( - 'externalizes %s as an absolute-path commonjs require', - async request => { - const external = await externalizeOrchestrionRuntimePackages({ request }); - - expect(external).toMatch(/^commonjs /); - const resolvedPath = external!.slice('commonjs '.length); - expect(isAbsolute(resolvedPath)).toBe(true); - expect(existsSync(resolvedPath)).toBe(true); - }, - ); + // An absolute path here breaks every deploy that relocates the output, so it has to stay bare. + it.each([ + ['@sentry/server-utils', '@sentry/nextjs/orchestrion-runtime/index'], + ['@sentry/server-utils/orchestrion', '@sentry/nextjs/orchestrion-runtime/orchestrion'], + ['@sentry/server-utils/orchestrion/register', '@sentry/nextjs/orchestrion-runtime/orchestrion/register'], + ['@sentry/server-utils/orchestrion/webpack', '@sentry/nextjs/orchestrion-runtime/orchestrion/webpack'], + ])('externalizes %s as the relocatable bare specifier %s', async (request, expected) => { + const external = await externalizeOrchestrionRuntimePackages({ request }); + + expect(external).toBe(`commonjs ${expected}`); + expect(isAbsolute(expected)).toBe(false); + }); it('ignores the bundled @apm-js-collab packages — no import of them exists in the dist anymore', async () => { await expect( @@ -74,12 +78,14 @@ describe('externalizeOrchestrionRuntimePackages', () => { ).resolves.toBeUndefined(); }); - it('resolves @sentry/server-utils subpaths to the CJS build, since the emitted external is a require()', async () => { - const external = await externalizeOrchestrionRuntimePackages({ - request: '@sentry/server-utils/orchestrion/register', - }); - - expect(external).toMatch(/[/\\]cjs[/\\]/); + // A `commonjs` external could never load these, so webpack gets to report them instead. + it('ignores subpaths @sentry/server-utils does not expose to require()', async () => { + await expect( + externalizeOrchestrionRuntimePackages({ request: '@sentry/server-utils/orchestrion/hook' }), + ).resolves.toBeUndefined(); + await expect( + externalizeOrchestrionRuntimePackages({ request: '@sentry/server-utils/does-not-exist' }), + ).resolves.toBeUndefined(); }); it('ignores unrelated requests so later externals handlers still run', async () => { @@ -92,6 +98,38 @@ describe('externalizeOrchestrionRuntimePackages', () => { }); }); +// Exercises the generated artifacts, so it needs the package built — as does this file's import of +// `@sentry/server-utils`. +describe('orchestrion runtime forwarders (generated)', () => { + const nodeRequire = createRequire(import.meta.url); + const forwarderDir = fileURLToPath(new URL('../../build/orchestrion-runtime/', import.meta.url)); + + /** Every `@sentry/server-utils` entrypoint that a `commonjs` external could load. */ + const requireableSubpaths = Object.entries( + (nodeRequire('@sentry/server-utils/package.json') as { exports: Record }).exports, + ) + .filter(([key, conditions]) => key !== './package.json' && conditions.require) + .map(([key]) => key); + + it.each(requireableSubpaths)('generates a forwarder for %s that re-exports it unchanged', subpath => { + const request = `@sentry/server-utils${subpath.slice(1)}`; + const forwarderFile = `${forwarderDir}${subpath === '.' ? 'index' : subpath.slice(2)}.js`; + + expect(existsSync(forwarderFile)).toBe(true); + expect(nodeRequire(forwarderFile)).toBe(nodeRequire(request)); + }); + + // The emitted specifier must resolve the way it will from a chunk: through the package exports. + it.each(requireableSubpaths)('exposes the forwarder for %s through the package exports', subpath => { + const specifier = getOrchestrionForwarderSpecifier( + `@sentry/server-utils${subpath.slice(1)}`, + '@sentry/server-utils', + ); + + expect(nodeRequire.resolve(specifier)).toBe(`${forwarderDir}${subpath === '.' ? 'index' : subpath.slice(2)}.js`); + }); +}); + describe('resolveBuildTimeInstrumentationOption', () => { const webpack: BundlerInfo = { isWebpack: true, isTurbopack: false, isTurbopackSupported: true }; const turbopack: BundlerInfo = { isWebpack: false, isTurbopack: true, isTurbopackSupported: true }; diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index 4f87e7cddcc8..b7d68015d863 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -727,7 +727,7 @@ describe('constructWebpackConfigFunction()', () => { }); describe('orchestrion runtime externals', () => { - it('prepends an externals handler that resolves runtime packages to absolute paths', async () => { + it('prepends an externals handler that forwards runtime packages through @sentry/nextjs', async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: serverWebpackConfig, @@ -738,8 +738,8 @@ describe('constructWebpackConfigFunction()', () => { const externals = finalWebpackConfig.externals as ((data: { request?: string }) => Promise)[]; expect(Array.isArray(externals)).toBe(true); - await expect(externals[0]({ request: '@sentry/server-utils/orchestrion/register' })).resolves.toMatch( - /^commonjs ([/\\]|[A-Za-z]:).*register\.js$/, + await expect(externals[0]({ request: '@sentry/server-utils/orchestrion/register' })).resolves.toBe( + 'commonjs @sentry/nextjs/orchestrion-runtime/orchestrion/register', ); await expect(externals[0]({ request: 'some-other-package' })).resolves.toBeUndefined(); }); diff --git a/packages/server-utils/src/orchestrion/bundler/resolve.ts b/packages/server-utils/src/orchestrion/bundler/resolve.ts index d4d81229dceb..5412d9af6cb3 100644 --- a/packages/server-utils/src/orchestrion/bundler/resolve.ts +++ b/packages/server-utils/src/orchestrion/bundler/resolve.ts @@ -28,13 +28,15 @@ export function getOrchestrionLoaderPath(): string { * own on-disk location — where the whole dependency graph always resolves, regardless of the * consuming app's install layout. Returns `undefined` when the request can't be resolved. * - * Bundler configs use this in two ways: - * - to emit absolute-path `commonjs` externals: a bare-specifier external emitted into a bundled - * chunk resolves from the chunk's output location at runtime, which fails under isolated - * installs (pnpm) where these packages are transitive dependencies; - * - as a build-time resolution fallback for the `@sentry/server-utils/orchestrion` import the - * module-injected snippet places INSIDE transformed `node_modules` files, which a bundler - * resolving from the importing file's location can't find under isolated installs either. + * BUILD-TIME resolver only: the path is handed back to the bundler to load and bundle, never + * emitted into the output. An absolute path in emitted output doesn't survive the build directory + * being relocated (Vercel, Docker, `output: 'standalone'`), so a consumer that needs one of these + * packages to stay EXTERNAL must emit a bare specifier instead — see `@sentry/nextjs`'s + * `externalizeOrchestrionRuntimePackages`. + * + * The specific case it covers: the `@sentry/server-utils/orchestrion` import the module-injected + * snippet places INSIDE transformed `node_modules` files, which a bundler resolving from the + * importing file's location can't find under isolated installs. */ export function resolveOrchestrionRuntimeRequest(request: string): string | undefined { try {