diff --git a/packages/server-runtime-injection/src/register.ts b/packages/server-runtime-injection/src/register.ts index 881da139417f..7c437e2980c0 100644 --- a/packages/server-runtime-injection/src/register.ts +++ b/packages/server-runtime-injection/src/register.ts @@ -29,44 +29,82 @@ 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. - * - * 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`. + * 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). * - * 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 → 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 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 warnTransformerUnavailable(moduleName: string): void { +function warnTransformFailed(moduleName: string, error: unknown): void { + const reason = error instanceof Error ? error.message : String(error); + + // 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) { + 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 +137,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..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 @@ -51,6 +53,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 ?? []; @@ -64,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(() => { @@ -77,13 +87,83 @@ 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 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') }); 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('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(); + + // `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', () => {