From 73ca40bb723608f95189269c2f1471a934047001 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 9 Sep 2026 13:18:43 +0200 Subject: [PATCH 1/3] fix(node): Distinguish isolated transform failures from bundling in the runtime warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime loader emitted the always-on "`@sentry/server-runtime-injection` was bundled ... loads uninstrumented" warning for ANY `TypeError` thrown while transforming a module, misattributing unrelated failures to bundling and pointing users at externalizing the package (which can break other setups). - Include the underlying error in the warning, so it is self-diagnosing rather than asserting a cause the reader cannot verify. - Only claim "bundled" for the transform pipeline itself going missing (`parse`/`generate is not a function` — the fingerprint of a bundler tree-shaking the vendored parser, which fails every module the same way), guarded by nothing having been instrumented yet. Any other transform `TypeError` (e.g. `transform is not a function`) gets a scoped "Could not instrument " message that points at reporting it, not changing the build. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../server-runtime-injection/src/register.ts | 78 ++++++++++++------- .../test/register.test.ts | 46 +++++++++++ 2 files changed, 98 insertions(+), 26 deletions(-) diff --git a/packages/server-runtime-injection/src/register.ts b/packages/server-runtime-injection/src/register.ts index 881da139417f..9efbf392cd34 100644 --- a/packages/server-runtime-injection/src/register.ts +++ b/packages/server-runtime-injection/src/register.ts @@ -29,44 +29,70 @@ function hasStableSyncModuleHooks(isDeno: boolean): boolean { } /** - * Emit a single, always-on warning that runtime channel injection is disabled, with the actionable - * fix. Unlike `debug.warn` (gated behind `debug: true`), this reaches every user — otherwise the - * SDK silently records no channel-based spans. + * Emit an always-on warning. Unlike `debug.warn` (gated behind `debug: true`), this reaches every + * user — otherwise a broken transform silently records no channel-based spans. */ -function warnRuntimeUnavailable(message: string): void { +function warn(message: string): void { consoleSandbox(() => { // oxlint-disable-next-line no-console - console.warn(`[Sentry] ${message} See ${BUNDLING_DOCS_URL}`); + console.warn(`[Sentry] ${message}`); }); } -// One broken transformer breaks every module, so state the fix once. +/** As {@link warn}, but appends the bundling/troubleshooting docs link for the build-fix cases. */ +function warnRuntimeUnavailable(message: string): void { + warn(`${message} See ${BUNDLING_DOCS_URL}`); +} + +// A systemic transformer failure breaks every module, so the "bundled" fix is stated once. let warnedTransformerUnavailable = false; +// Isolated per-module failures are unrelated to bundling, so they warn once per module. +const warnedModuleFailures = new Set(); /** - * Warn that the vendored code transformer could not run, so `moduleName` loaded uninstrumented. + * A module's transform threw. Two very different causes reach this callback, so distinguish them + * instead of always blaming bundling — and always include the underlying error, so the message is + * self-diagnosing rather than asserting a cause the reader can't check (`debug: true` still logs + * the full error and stack). * - * This package ships the transformer (meriyah/astring/source-map) inline and is meant to run from - * `node_modules`. A bundler that inlines and tree-shakes `@sentry/server-runtime-injection` strips it, so every - * transform throws `TypeError: parse is not a function` — swallowed inside the loader, once per - * module, visible only with `debug: true`. - * - * Warning from here rather than probing the transformer at `init()` keeps the check honest. A - * module only reaches this callback by coming through Node's loader, which means the build-time - * bundler plugin did not cover it, which means the instrumentation really is lost. Probing at - * `init()` instead has to guess at that from a global the plugin's entry banner may not have - * written yet. + * - The transform pipeline itself is gone → `TypeError: parse is not a function` / + * `generate is not a function`. That is the fingerprint of a bundler inlining and tree-shaking + * `@sentry/server-runtime-injection`, which strips its vendored meriyah/astring parser: it fails + * EVERY module the same way and is app-wide, with an actionable build fix. (As a second guard, a + * transformer that has already instrumented something is provably not stripped.) + * - Any other transform `TypeError` (e.g. `transform is not a function`, from a config whose + * operator is not registered at runtime) is not a bundling problem — even when it is the first + * module to load — so it gets a scoped message that points at reporting it, not changing the build. */ -function warnTransformerUnavailable(moduleName: string): void { +function warnTransformFailed(moduleName: string, error: unknown): void { + const reason = error instanceof Error ? error.message : String(error); + + // Only the parser/generator primitives going missing means the transformer was stripped; a + // non-empty `runtime` list (something was already instrumented) proves it was not. + const pipelineStripped = /\b(?:parse|generate) is not a function\b/.test(reason); + const nothingInstrumentedYet = (GLOBAL_OBJ.__SENTRY_ORCHESTRION__?.runtime?.length ?? 0) === 0; + + if (!(pipelineStripped && nothingInstrumentedYet)) { + if (warnedModuleFailures.has(moduleName)) { + return; + } + warnedModuleFailures.add(moduleName); + warn( + `Could not instrument \`${moduleName}\` (${reason}). Other instrumented dependencies are ` + + `unaffected, so this is not a bundling problem. If \`${moduleName}\` should be traced, please report it.`, + ); + return; + } + if (warnedTransformerUnavailable) { return; } warnedTransformerUnavailable = true; - warnRuntimeUnavailable( - `\`@sentry/server-runtime-injection\` was bundled into your application, so ${moduleName} and any other ` + - 'instrumented dependency load uninstrumented. Keep `@sentry/server-runtime-injection` external in your ' + - 'server bundle, or use the Sentry bundler plugin for build-time instrumentation.', + `\`@sentry/server-runtime-injection\` was bundled into your application, so \`${moduleName}\` and any ` + + `other instrumented dependency load uninstrumented (${reason}). Keep ` + + '`@sentry/server-runtime-injection` external in your server bundle, or use the Sentry bundler ' + + 'plugin for build-time instrumentation.', ); } @@ -99,11 +125,11 @@ export function registerDiagnosticsChannelInjection(): void { setDiagnosticsHook(({ url, moduleName, error }): void => { if (error) { - // A stripped transformer surfaces as a `TypeError` (`parse`/`generate` are `undefined`) and - // costs the user this module's instrumentation, so it is worth an always-on warning. Every - // other transform failure stays debug-only. + // A transform throwing a `TypeError` costs this module its instrumentation, so it is worth an + // always-on warning; `warnTransformFailed` decides whether that is systemic (bundling) or + // isolated. Every other transform failure stays debug-only. if (error instanceof TypeError) { - warnTransformerUnavailable(moduleName); + warnTransformFailed(moduleName, error); } debug.warn(`[instrumentation] failed to inject diagnostics-channel into ${moduleName}:`, error); } else { diff --git a/packages/server-runtime-injection/test/register.test.ts b/packages/server-runtime-injection/test/register.test.ts index 2170f3b4de8d..b82e0eb7147b 100644 --- a/packages/server-runtime-injection/test/register.test.ts +++ b/packages/server-runtime-injection/test/register.test.ts @@ -51,6 +51,11 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', return warnSpy.mock.calls.map(([message]) => String(message)).filter(m => m.includes('was bundled into')); } + /** The isolated per-module failure warnings only (a transform failure that is not a stripped transformer). */ + function moduleFailureWarnings(): string[] { + return warnSpy.mock.calls.map(([message]) => String(message)).filter(m => m.includes('Could not instrument')); + } + /** The callback the registration handed to tracing-hooks. */ function diagnosticsCallback(): DiagnosticsCallback { const [callback] = setDiagnosticsHookMock.mock.lastCall ?? []; @@ -84,6 +89,47 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', expect(bundlingWarnings()).toHaveLength(1); expect(bundlingWarnings()[0]).toContain('mysql2'); expect(bundlingWarnings()[0]).toContain('docs.sentry.io'); + // The underlying error is surfaced so the warning is self-diagnosing. + expect(bundlingWarnings()[0]).toContain('parse is not a function'); + }); + + it('reports a non-stripped transform TypeError as an isolated failure, not bundling', () => { + registerDiagnosticsChannelInjection(); + + // `transform is not a function` (a config operator not registered at runtime) is not the + // stripped-parser fingerprint, so even as the first and only failing module it must not be + // blamed on bundling. + diagnosticsCallback()({ moduleName: 'lib-op-missing', error: new TypeError('transform is not a function') }); + + expect(bundlingWarnings()).toEqual([]); + expect(moduleFailureWarnings()).toHaveLength(1); + expect(moduleFailureWarnings()[0]).toContain('lib-op-missing'); + expect(moduleFailureWarnings()[0]).toContain('transform is not a function'); + }); + + it('treats a stripped-parser error as isolated once another module has been instrumented', () => { + registerDiagnosticsChannelInjection(); + const onDiagnostics = diagnosticsCallback(); + + // A successful transform proves the transformer works, so a later `parse is not a function` + // cannot be a tree-shaken transformer — it is isolated to that module. + onDiagnostics({ moduleName: 'lib-ok' }); + onDiagnostics({ moduleName: 'lib-guarded', error: new TypeError('parse is not a function') }); + + expect(bundlingWarnings()).toEqual([]); + expect(moduleFailureWarnings()).toHaveLength(1); + expect(moduleFailureWarnings()[0]).toContain('lib-guarded'); + }); + + it('warns once per module for isolated failures', () => { + registerDiagnosticsChannelInjection(); + const onDiagnostics = diagnosticsCallback(); + + onDiagnostics({ moduleName: 'lib-dup', error: new TypeError('transform is not a function') }); + onDiagnostics({ moduleName: 'lib-dup', error: new TypeError('transform is not a function') }); + onDiagnostics({ moduleName: 'lib-other', error: new TypeError('transform is not a function') }); + + expect(moduleFailureWarnings()).toHaveLength(2); }); it('stays quiet for transform failures that are not a stripped transformer', () => { From 64ba1e852131f3664b8ccd4c54db2c84334e3c81 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 9 Sep 2026 14:14:15 +0200 Subject: [PATCH 2/3] fixes and stuff --- packages/server-runtime-injection/src/register.ts | 13 ++++++++----- .../server-runtime-injection/test/register.test.ts | 10 ++++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/server-runtime-injection/src/register.ts b/packages/server-runtime-injection/src/register.ts index 9efbf392cd34..b900dd159295 100644 --- a/packages/server-runtime-injection/src/register.ts +++ b/packages/server-runtime-injection/src/register.ts @@ -57,19 +57,22 @@ const warnedModuleFailures = new Set(); * * - The transform pipeline itself is gone → `TypeError: parse is not a function` / * `generate is not a function`. That is the fingerprint of a bundler inlining and tree-shaking - * `@sentry/server-runtime-injection`, which strips its vendored meriyah/astring parser: it fails - * EVERY module the same way and is app-wide, with an actionable build fix. (As a second guard, a + * `@sentry/server-runtime-injection`, which strips its vendored meriyah `parse` / astring + * `generate`: it fails EVERY module the same way and is app-wide, with an actionable build fix. + * Those are module-level imports, so the same bundler renames them while merging modules (esbuild + * emits `parse2`/`generate3`), hence the numeric-suffix tolerance. (As a second guard, a * transformer that has already instrumented something is provably not stripped.) * - Any other transform `TypeError` (e.g. `transform is not a function`, from a config whose - * operator is not registered at runtime) is not a bundling problem — even when it is the first - * module to load — so it gets a scoped message that points at reporting it, not changing the build. + * operator is not registered at runtime — `transform` is a local, not a stripped import) is not a + * bundling problem — even when it is the first module to load — so it gets a scoped message that + * points at reporting it, not changing the build. */ function warnTransformFailed(moduleName: string, error: unknown): void { const reason = error instanceof Error ? error.message : String(error); // Only the parser/generator primitives going missing means the transformer was stripped; a // non-empty `runtime` list (something was already instrumented) proves it was not. - const pipelineStripped = /\b(?:parse|generate) is not a function\b/.test(reason); + const pipelineStripped = /\b(?:parse|generate)\d* is not a function\b/.test(reason); const nothingInstrumentedYet = (GLOBAL_OBJ.__SENTRY_ORCHESTRION__?.runtime?.length ?? 0) === 0; if (!(pipelineStripped && nothingInstrumentedYet)) { diff --git a/packages/server-runtime-injection/test/register.test.ts b/packages/server-runtime-injection/test/register.test.ts index b82e0eb7147b..88688d33b887 100644 --- a/packages/server-runtime-injection/test/register.test.ts +++ b/packages/server-runtime-injection/test/register.test.ts @@ -82,15 +82,17 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', registerDiagnosticsChannelInjection(); const onDiagnostics = diagnosticsCallback(); - // A tree-shaken chain: `parse`/`generate` are `undefined`, so the transform throws a TypeError. - onDiagnostics({ moduleName: 'mysql2', error: new TypeError('parse is not a function') }); - onDiagnostics({ moduleName: 'pg', error: new TypeError('parse is not a function') }); + // A tree-shaken chain: the vendored `parse`/`generate` are `undefined`, so the transform throws + // a TypeError. The bundler that stripped them also renames these module-level imports while + // merging modules — esbuild emits `parse3` — so the fingerprint has to tolerate the suffix. + onDiagnostics({ moduleName: 'mysql2', error: new TypeError('parse3 is not a function') }); + onDiagnostics({ moduleName: 'pg', error: new TypeError('parse3 is not a function') }); expect(bundlingWarnings()).toHaveLength(1); expect(bundlingWarnings()[0]).toContain('mysql2'); expect(bundlingWarnings()[0]).toContain('docs.sentry.io'); // The underlying error is surfaced so the warning is self-diagnosing. - expect(bundlingWarnings()[0]).toContain('parse is not a function'); + expect(bundlingWarnings()[0]).toContain('parse3 is not a function'); }); it('reports a non-stripped transform TypeError as an isolated failure, not bundling', () => { From 9e591ae308e0f52eed92edb96f189c9ebdd8a600 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 9 Sep 2026 15:37:30 +0200 Subject: [PATCH 3/3] fix(node): Detect tree-shaken transform pipeline regardless of bundler renaming The stripped-transformer fingerprint matched `parse`/`generate` with an optional digit suffix, which only covers esbuild's numeric deconfliction (`parse2`). Rollup/Vite deconflict with a `$N` suffix (`parse$1`) and production minifiers rename to short opaque names (`e`), so a genuinely bundled pipeline fell through to the isolated branch and told users "this is not a bundling problem". Match the shape of the error (a bare ` is not a function`) plus the fact that nothing was ever instrumented, rather than the primitive name. Keep `transform is not a function` as the explicit isolated carve-out (a local dispatch read under its real name from an unbundled install) and anchor the regex so member-expression failures (`x.y is not a function`) stay isolated. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../server-runtime-injection/src/register.ts | 37 +++++++++++------- .../test/register.test.ts | 38 +++++++++++++++++-- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/packages/server-runtime-injection/src/register.ts b/packages/server-runtime-injection/src/register.ts index b900dd159295..7c437e2980c0 100644 --- a/packages/server-runtime-injection/src/register.ts +++ b/packages/server-runtime-injection/src/register.ts @@ -55,27 +55,36 @@ const warnedModuleFailures = new Set(); * self-diagnosing rather than asserting a cause the reader can't check (`debug: true` still logs * the full error and stack). * - * - The transform pipeline itself is gone → `TypeError: parse is not a function` / - * `generate is not a function`. That is the fingerprint of a bundler inlining and tree-shaking - * `@sentry/server-runtime-injection`, which strips its vendored meriyah `parse` / astring - * `generate`: it fails EVERY module the same way and is app-wide, with an actionable build fix. - * Those are module-level imports, so the same bundler renames them while merging modules (esbuild - * emits `parse2`/`generate3`), hence the numeric-suffix tolerance. (As a second guard, a - * transformer that has already instrumented something is provably not stripped.) + * - The transform pipeline itself is gone → a bare `TypeError: is not a function`. That is + * the fingerprint of a bundler inlining and tree-shaking `@sentry/server-runtime-injection`, + * which drops its vendored meriyah `parse` / astring `generate` (both module-level imports): it + * fails EVERY module the same way and is app-wide, with an actionable build fix. The identifier is + * NOT matched, because the same bundle renames it beyond recognition — esbuild deconflicts to + * `parse2`, rollup/vite to `parse$1`, and production minifiers to a short opaque `n` — so we match + * the *shape* (a bare ` is not a function`) plus the fact that nothing was ever + * instrumented (a transformer that has already instrumented something is provably not stripped). * - Any other transform `TypeError` (e.g. `transform is not a function`, from a config whose - * operator is not registered at runtime — `transform` is a local, not a stripped import) is not a - * bundling problem — even when it is the first module to load — so it gets a scoped message that - * points at reporting it, not changing the build. + * operator is not registered at runtime — `transform` is a local dispatch, not a stripped import, + * so an unbundled install throws with its real, unmangled name) is not a bundling problem — even + * when it is the first module to load — so it gets a scoped message that points at reporting it, + * not changing the build. A member-expression failure (`x.y is not a function`) is likewise an + * ordinary per-module bug, not a stripped top-level binding, so the bare-identifier anchor + * excludes it. */ function warnTransformFailed(moduleName: string, error: unknown): void { const reason = error instanceof Error ? error.message : String(error); - // Only the parser/generator primitives going missing means the transformer was stripped; a - // non-empty `runtime` list (something was already instrumented) proves it was not. - const pipelineStripped = /\b(?:parse|generate)\d* is not a function\b/.test(reason); + // A stripped pipeline throws ` is not a function` for a *bare* identifier (the renamed + // `parse`/`generate` binding); the anchors exclude member-expression failures like `x.y is ...`. + const barePrimitiveMissing = /^[\w$]+ is not a function$/.test(reason); + // The operator dispatch is a local, only missing when a config operator is unregistered, and an + // unbundled install reads it under its real name — never a stripped-transformer symptom. + const isolatedOperatorFailure = reason === 'transform is not a function'; + // A non-empty `runtime` list (something was already instrumented) proves the pipeline works. const nothingInstrumentedYet = (GLOBAL_OBJ.__SENTRY_ORCHESTRION__?.runtime?.length ?? 0) === 0; + const pipelineStripped = barePrimitiveMissing && !isolatedOperatorFailure && nothingInstrumentedYet; - if (!(pipelineStripped && nothingInstrumentedYet)) { + if (!pipelineStripped) { if (warnedModuleFailures.has(moduleName)) { return; } diff --git a/packages/server-runtime-injection/test/register.test.ts b/packages/server-runtime-injection/test/register.test.ts index 88688d33b887..18307c0073eb 100644 --- a/packages/server-runtime-injection/test/register.test.ts +++ b/packages/server-runtime-injection/test/register.test.ts @@ -36,12 +36,14 @@ vi.mock('@sentry/core', async importOriginal => { }); import { GLOBAL_OBJ } from '@sentry/core'; -import { registerDiagnosticsChannelInjection } from '../src/register'; +import type * as RegisterModule from '../src/register'; type DiagnosticsCallback = (event: { moduleName: string; error?: unknown }) => void; describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', () => { let warnSpy: ReturnType; + // Re-imported per test (see `beforeEach`) so the one-warning-per-process module state is fresh. + let registerDiagnosticsChannelInjection: typeof RegisterModule.registerDiagnosticsChannelInjection; /** * The bundling warnings only. Registration itself can warn for unrelated reasons in this @@ -69,8 +71,11 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', delete GLOBAL_OBJ.__SENTRY_ORCHESTRION__; setDiagnosticsHookMock.mockClear(); warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); - // The one-warning-per-process flag is module state, so each test needs a fresh module. + // The one-warning-per-process flags are module state, so each test needs a fresh module — hence + // the dynamic re-import below rather than a static top-level one (`resetModules` cannot refresh + // an already-bound static import). vi.resetModules(); + ({ registerDiagnosticsChannelInjection } = await import('../src/register')); }); afterEach(() => { @@ -84,7 +89,7 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', // A tree-shaken chain: the vendored `parse`/`generate` are `undefined`, so the transform throws // a TypeError. The bundler that stripped them also renames these module-level imports while - // merging modules — esbuild emits `parse3` — so the fingerprint has to tolerate the suffix. + // merging modules — esbuild emits `parse3` — so the fingerprint must not hinge on the name. onDiagnostics({ moduleName: 'mysql2', error: new TypeError('parse3 is not a function') }); onDiagnostics({ moduleName: 'pg', error: new TypeError('parse3 is not a function') }); @@ -95,6 +100,33 @@ describe('registerDiagnosticsChannelInjection - bundled/tree-shaken detection', expect(bundlingWarnings()[0]).toContain('parse3 is not a function'); }); + it.each([ + // Rollup/Vite deconflict merged bindings with a `$N` suffix, not a bare number. + ['rollup/vite `$N` suffix', 'generate$1 is not a function'], + // Production minifiers rename the stripped import to a short opaque identifier. + ['minified short name', 'e is not a function'], + ])('detects a tree-shaken transformer whose primitive was renamed (%s)', (_label, reason) => { + registerDiagnosticsChannelInjection(); + + diagnosticsCallback()({ moduleName: 'mysql2', error: new TypeError(reason) }); + + expect(bundlingWarnings()).toHaveLength(1); + expect(bundlingWarnings()[0]).toContain('mysql2'); + expect(bundlingWarnings()[0]).toContain(reason); + }); + + it('treats a member-expression TypeError as an isolated failure, not bundling', () => { + registerDiagnosticsChannelInjection(); + + // `x.y is not a function` is an ordinary per-module bug, not a stripped bare top-level binding, + // so even as the first failing module it must not be blamed on bundling. + diagnosticsCallback()({ moduleName: 'lib-member', error: new TypeError('node.foo is not a function') }); + + expect(bundlingWarnings()).toEqual([]); + expect(moduleFailureWarnings()).toHaveLength(1); + expect(moduleFailureWarnings()[0]).toContain('lib-member'); + }); + it('reports a non-stripped transform TypeError as an isolated failure, not bundling', () => { registerDiagnosticsChannelInjection();