diff --git a/dev-packages/e2e-tests/test-applications/nextjs-16-standalone/tests/build-output.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-16-standalone/tests/build-output.test.ts index 5384a96579c6..936ff77b6494 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-16-standalone/tests/build-output.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-16-standalone/tests/build-output.test.ts @@ -1,5 +1,6 @@ import { expect, test } from '@playwright/test'; import { findAbsolutePathImports } from '@sentry-internal/test-utils'; +import * as fs from 'fs'; import * as path from 'path'; // `output: 'standalone'` is the mode where a baked-in absolute specifier actually bites: the server @@ -12,3 +13,20 @@ test('emits no absolute-path imports into the relocated standalone output', () = expect(leaks).toEqual([]); }); + +// Regression test for the bug this app surfaced (JS-3451): `register.ts` reaches the ESM loader +// hook through `Module.register(...)`, a runtime call `@vercel/nft` cannot trace. The hook was +// therefore never copied into the standalone output, and channel-based instrumentation failed to +// register, silently, behind `debug: true`. Asserting on the traced output rather than on emitted +// telemetry, because Next.js records its server spans through its own wrapping either way, so the +// tests in `standalone.test.ts` stay green even when channel injection is dead. +test('traces the orchestrion ESM loader hook into the standalone output', () => { + const standaloneDir = path.join(process.cwd(), '.next', 'standalone'); + const hookSuffix = path.join('@sentry', 'server-runtime-injection', 'build', 'esm', 'hook.js'); + + const found = fs + .readdirSync(standaloneDir, { recursive: true, withFileTypes: true }) + .some(entry => entry.isFile() && path.join(entry.parentPath, entry.name).endsWith(hookSuffix)); + + expect(found).toBe(true); +}); diff --git a/packages/nextjs/src/config/diagnosticsChannelInjection.ts b/packages/nextjs/src/config/diagnosticsChannelInjection.ts index 36939c446f2b..ffc215061eda 100644 --- a/packages/nextjs/src/config/diagnosticsChannelInjection.ts +++ b/packages/nextjs/src/config/diagnosticsChannelInjection.ts @@ -11,10 +11,10 @@ export const BUNDLE_SAFE_INSTRUMENTED_PACKAGES = ['ioredis']; /** * `@sentry/server-runtime-injection` (where `register.ts` and the bundled orchestrion runtime ship) * must stay external: `register.ts` passes its own `__filename`/`import.meta.url` as the `parentURL` - * for `Module.register('@sentry/server-runtime-injection/hook', …)`, so that self-reference only - * resolves while the code still lives at its real `node_modules` location. Bundled into an app - * server chunk instead, the specifier would have to resolve from the chunk's output location, - * which fails under isolated installs (pnpm) where the package is a transitive dependency. + * for `Module.register()` and resolves the ESM loader hook relative to that same location, so both + * only work while the code still lives at its real `node_modules` location. Bundled into an app + * server chunk instead, they would resolve from the chunk's output location, where neither the hook + * nor the vendored transformer it loads exists. * * `@sentry/server-utils` (the barrel + bundler plugins) is NOT here — it is meant to be bundled; the * build-time snippet's `@sentry/server-utils` import is handled separately by the code-transform. diff --git a/packages/server-runtime-injection/package.json b/packages/server-runtime-injection/package.json index 48492ad26393..d78b70484685 100644 --- a/packages/server-runtime-injection/package.json +++ b/packages/server-runtime-injection/package.json @@ -52,6 +52,7 @@ "@apm-js-collab/code-transformer": "^0.18.1", "@apm-js-collab/tracing-hooks": "^0.13.0", "@types/node": "^18.19.1", + "@vercel/nft": "^1.3.0", "meriyah": "^6.1.4" }, "scripts": { diff --git a/packages/server-runtime-injection/src/register.ts b/packages/server-runtime-injection/src/register.ts index 288bc571a6f5..fc912731befc 100644 --- a/packages/server-runtime-injection/src/register.ts +++ b/packages/server-runtime-injection/src/register.ts @@ -1,6 +1,8 @@ import { consoleSandbox, debug, getClient, GLOBAL_OBJ, parseSemver } from '@sentry/core'; +import { existsSync } from 'node:fs'; import * as Module from 'node:module'; -import { pathToFileURL } from 'node:url'; +import { dirname, join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import { SENTRY_INSTRUMENTATIONS } from '@sentry/server-utils/orchestrion/config'; import type { register } from 'node:module'; import ModulePatch from '@apm-js-collab/tracing-hooks'; @@ -140,9 +142,45 @@ export function registerDiagnosticsChannelInjection(): void { // Our own bundled copy of the tracing-hooks async hooks (see `src/hook.mjs`) — the dependency // itself is bundled into this package's build and no longer resolvable as a bare specifier at - // runtime. This self-referential specifier only resolves while this package lives at its real - // `node_modules` location, which is why it must stay external (never bundled into an app). - mod.register('@sentry/server-runtime-injection/hook', { + // runtime. + // + // Registered by path rather than through the `@sentry/server-runtime-injection/hook` + // self-reference, because `Module.register` resolves its specifier at RUNTIME. `@vercel/nft` + // does evaluate `Module.register()` calls, but only where it can follow the `node:module` + // binding, and in the CJS build rollup routes that through an interop namespace helper + // (`const mod = require$$1__namespace`) it cannot see through. The hook then never reaches + // traced output, and the CJS build is the one Next.js loads, so `output: 'standalone'`, + // Docker and Vercel builds lose channel instrumentation entirely and only say so behind + // `debug: true`. A literal relative path is static in either build, and it is still computed + // at runtime from `__filename`/`import.meta.url`, so nothing absolute is baked in. + // + // Built from `join()` rather than `new URL('./hook.js', import.meta.url)` because webpack + // reads that second form as an asset reference: it copies the hook next to the app bundle + // without its vendored chunks, which would leave the loader thread importing a file whose + // own imports are missing. + let hookPath: string; + /*! rollup-include-cjs-only */ + // This file is `build/cjs/register.js`; the loader thread needs the ESM build, which shares + // the vendored dependency chunks. + hookPath = join(__dirname, '../esm/hook.js'); + /*! rollup-include-cjs-only-end */ + /*! rollup-include-esm-only */ + hookPath = join(dirname(fileURLToPath(import.meta.url)), 'hook.js'); + /*! rollup-include-esm-only-end */ + + // The path only points at the shipped hook while this package runs from `node_modules`. A + // copy bundled into an app sits somewhere else entirely, so it keeps the self-reference, + // which at least resolves against the app's own install. The same fallback catches a broken + // path after a build layout change, hence the log: tracers would quietly stop following the + // hook again, and this line is the only thing that says so. + const hookFound = existsSync(hookPath); + if (!hookFound) { + debug.warn(`No orchestrion ESM hook at ${hookPath}; falling back to the package specifier.`); + } + + const hookSpecifier = hookFound ? pathToFileURL(hookPath).href : '@sentry/server-runtime-injection/hook'; + + mod.register(hookSpecifier, { parentURL, data: { instrumentations: SENTRY_INSTRUMENTATIONS, diagnosticsPort }, transferList: [diagnosticsPort], diff --git a/packages/server-runtime-injection/test/nftTrace.test.ts b/packages/server-runtime-injection/test/nftTrace.test.ts new file mode 100644 index 000000000000..2aa8ff749dbe --- /dev/null +++ b/packages/server-runtime-injection/test/nftTrace.test.ts @@ -0,0 +1,34 @@ +import { join } from 'node:path'; +import { nodeFileTrace } from '@vercel/nft'; +import { describe, expect, it } from 'vitest'; + +/** + * `Module.register()` resolves its specifier at runtime. nft does evaluate those calls, but it lost + * the hook in the CJS build, where rollup reaches `node:module` through an interop namespace helper + * it cannot follow, and that is the build Next.js loads. `output: 'standalone'`, Docker and Vercel + * builds then leave the hook out and drop channel-based instrumentation, saying so only behind + * `debug: true`. + * + * Runs the real `@vercel/nft` over the emitted entry points, because the guarantee is a property of + * the built files, not of the source: it needs the package built. + */ +const repoRoot = join(__dirname, '..', '..', '..'); +const buildDir = 'packages/server-runtime-injection/build'; + +async function traceFrom(entry: string): Promise> { + const { fileList } = await nodeFileTrace([join(repoRoot, entry)], { base: repoRoot }); + return fileList; +} + +describe.each(['cjs', 'esm'])('the %s build', variant => { + it('traces the ESM loader hook and the chunks it imports', { timeout: 60_000 }, async () => { + const traced = await traceFrom(`${buildDir}/${variant}/register.js`); + + expect(traced).toContain(`${buildDir}/esm/hook.js`); + // The hook is ESM. Without this marker the loader thread parses it as CommonJS and fails. + expect(traced).toContain(`${buildDir}/esm/package.json`); + // The hook re-exports the vendored chain, so its presence alone would not prove the tracer + // followed the hook's own imports rather than copying it as an opaque asset. + expect(traced).toContain(`${buildDir}/esm/vendored/@apm-js-collab/tracing-hooks/hook.js`); + }); +});