From 8b3cbd36bad11da886fcce1045024a05127e8c40 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 21 Sep 2026 11:50:27 +0300 Subject: [PATCH 1/4] ref(cloudflare): Extract one factory for the Vite provider plugins `flueRuntime.ts` and `mastraObservability.ts` were near-identical, differing only in the module specifier, the injected identifier, the target regex, how they handled a failed resolve, and getter versus assignment. Both call sites are now a few lines over a shared `createProvidedModulePlugin`. Replace the `createRequire().resolve()` probe with the Rollup context's `this.resolve()`, which answers with the same resolver and conditions the injected import will use. That drops `createRequire`, `node:path` and the error-code special case, and it fixes a latent bug on the Mastra side: its bare `catch { return; }` worked only because `@mastra/observability` still publishes a `require` condition, so an ESM-only release would have turned injection off with no error and no log. The probe needs a plugin context, so it moves from `configResolved` to `buildStart`. `configResolved` stays to capture the app root. Resolution runs per environment against a shared plugin instance, so the probe stops once it finds the package and retries in the next environment otherwise. Only the worker environment ever reaches `transform`, and it may not run first. Mastra also picks up the guards Flue gained in #24476: `transform` is idempotent, a resolver error injects rather than silently skipping, and the namespace now goes behind the same lazy getter. Assigning reads the binding at injection time, so it stores `undefined` whenever the bundler evaluates Sentry's module first. That hazard is not specific to Flue, so both providers use one shape and the `lazy` option is gone. Co-Authored-By: Claude Opus 5 --- packages/cloudflare/src/vite/flueRuntime.ts | 71 +------- .../src/vite/mastraObservability.ts | 74 ++------- .../src/vite/providedModulePlugin.ts | 114 +++++++++++++ .../cloudflare/test/vite/flueRuntime.test.ts | 157 ++---------------- .../test/vite/mastraObservability.test.ts | 57 +++---- .../test/vite/providedModulePlugin.test.ts | 157 ++++++++++++++++++ 6 files changed, 340 insertions(+), 290 deletions(-) create mode 100644 packages/cloudflare/src/vite/providedModulePlugin.ts create mode 100644 packages/cloudflare/test/vite/providedModulePlugin.test.ts diff --git a/packages/cloudflare/src/vite/flueRuntime.ts b/packages/cloudflare/src/vite/flueRuntime.ts index 1ac2d09aed77..004012a5f09f 100644 --- a/packages/cloudflare/src/vite/flueRuntime.ts +++ b/packages/cloudflare/src/vite/flueRuntime.ts @@ -1,23 +1,5 @@ -import { createRequire } from 'node:module'; -import { resolve } from 'node:path'; -import MagicString from 'magic-string'; - -// Namespace binding the injected provider import uses; read back by the integration -// off the global marker. -const PROVIDER_IDENTIFIER = '__SENTRY_FLUE_RUNTIME__'; - -const FLUE_MODULE = '@flue/runtime'; - -// The bundled `@sentry/server-utils` Flue integration module (ESM build — the only one a -// worker loads). It reads `@flue/runtime` off the global marker this provider populates, -// because `instrument()` registers into module-scope state no channel payload can carry. -const FLUE_INTEGRATION_ID = /@sentry\/server-utils\/build\/esm\/integrations\/flue\.js$/; - -/** Whether `id` is the Sentry Flue integration module the provider injects into. */ -export function isFlueIntegrationModuleId(id: string): boolean { - const normalizedId = id.replace(/\\/g, '/').replace(/[?#].*$/, ''); - return FLUE_INTEGRATION_ID.test(normalizedId); -} +import type { ProvidedModulePlugin } from './providedModulePlugin'; +import { createProvidedModulePlugin } from './providedModulePlugin'; /** * Splices a static `import * as … from '@flue/runtime'` into Sentry's own Flue integration module @@ -28,48 +10,11 @@ export function isFlueIntegrationModuleId(id: string): boolean { * user passes it by calling `instrument()` themselves; a bundled worker has no `node_modules` to * resolve from, so it is supplied at build time instead. */ -export function sentryFlueRuntimeProviderPlugin(): { - name: string; - configResolved(config: { root: string }): void; - transform(code: string, id: string): { code: string; map: ReturnType } | undefined; -} { - let providerSnippet: string | undefined; - - return { +export function sentryFlueRuntimeProviderPlugin(): ProvidedModulePlugin { + return createProvidedModulePlugin({ name: 'sentry-cloudflare-flue-runtime-provider', - - configResolved(config: { root: string }): void { - // Build-time only; never ships to the worker. Probed with CJS resolution, which an ESM-only - // `@flue/runtime` fails with `ERR_PACKAGE_PATH_NOT_EXPORTED` — so only a module-not-found - // counts as absent, and any other failure still injects and lets Vite report it. Not - // `import.meta.resolve`: `parentURL` is ignored without a flag, and it is absent from the - // CJS build. - try { - createRequire(resolve(config.root, 'noop.js')).resolve(FLUE_MODULE); - } catch (error) { - const code = (error as NodeJS.ErrnoException | undefined)?.code; - if (code === 'MODULE_NOT_FOUND' || code === 'ERR_MODULE_NOT_FOUND') { - return; - } - } - // A getter where Mastra assigns: the bundler may evaluate Sentry's module before - // `@flue/runtime` is initialized, and assigning there would store `undefined`. - providerSnippet = - `import * as ${PROVIDER_IDENTIFIER} from '${FLUE_MODULE}';\n` + - '(globalThis.__SENTRY_ORCHESTRION__ = globalThis.__SENTRY_ORCHESTRION__ || {});\n' + - '(globalThis.__SENTRY_ORCHESTRION__.providedModules = globalThis.__SENTRY_ORCHESTRION__.providedModules || {});\n' + - `Object.defineProperty(globalThis.__SENTRY_ORCHESTRION__.providedModules, '${FLUE_MODULE}', ` + - `{ configurable: true, enumerable: true, get() { return ${PROVIDER_IDENTIFIER}; } });\n`; - }, - - transform(code: string, id: string): { code: string; map: ReturnType } | undefined { - // `code.includes` keeps this idempotent: a second pass over already-injected output would - // otherwise emit a duplicate `import * as` binding, which is a syntax error. - if (!providerSnippet || !isFlueIntegrationModuleId(id) || code.includes(PROVIDER_IDENTIFIER)) return undefined; - - const ms = new MagicString(code); - ms.prepend(providerSnippet); - return { code: ms.toString(), map: ms.generateMap({ hires: true }) }; - }, - }; + moduleName: '@flue/runtime', + identifier: '__SENTRY_FLUE_RUNTIME__', + integrationModule: 'flue', + }); } diff --git a/packages/cloudflare/src/vite/mastraObservability.ts b/packages/cloudflare/src/vite/mastraObservability.ts index a77d5037b965..67f225ad05f1 100644 --- a/packages/cloudflare/src/vite/mastraObservability.ts +++ b/packages/cloudflare/src/vite/mastraObservability.ts @@ -1,66 +1,20 @@ -import { createRequire } from 'node:module'; -import { resolve } from 'node:path'; -import MagicString from 'magic-string'; - -// Namespace binding the injected provider import uses; read back by the integration -// off the global marker. -const PROVIDER_IDENTIFIER = '__SENTRY_MASTRA_OBSERVABILITY__'; - -// The bundled `@sentry/server-utils` Mastra integration module (ESM build — the only -// one a worker loads). Its `loadMastraObservability` reads `@mastra/observability` off -// the global marker this provider populates, instead of `createRequire`, which cannot -// resolve a package inside a bundled worker. -const MASTRA_INTEGRATION_ID = /@sentry\/server-utils\/build\/esm\/integrations\/mastra\.js$/; - -/** Whether `id` is the Sentry Mastra integration module the provider injects into. */ -export function isMastraIntegrationModuleId(id: string): boolean { - const normalizedId = id.replace(/\\/g, '/').replace(/[?#].*$/, ''); - return MASTRA_INTEGRATION_ID.test(normalizedId); -} +import type { ProvidedModulePlugin } from './providedModulePlugin'; +import { createProvidedModulePlugin } from './providedModulePlugin'; /** - * Splices a static `import * as … from '@mastra/observability'` into Sentry's own - * Mastra integration module and stashes the namespace on the global orchestrion - * marker. + * Splices a static `import * as … from '@mastra/observability'` into Sentry's own Mastra + * integration module and stashes the namespace on the global orchestrion marker. * - * On Cloudflare the integration cannot `createRequire('@mastra/observability')` to - * bootstrap Mastra's observability pipeline — there is no on-disk `node_modules` in - * workerd — so without this the user has to construct and wire up an `Observability` - * themselves. The import is static (statically analyzable, no lazy `import()`), lands - * in Sentry's module rather than the user's code, and is only emitted when the package - * actually resolves; if it is absent, the integration keeps its Node `createRequire` - * fallback and the marker stays empty. + * On Cloudflare the integration cannot `createRequire('@mastra/observability')` to bootstrap + * Mastra's observability pipeline — there is no on-disk `node_modules` in workerd — so without + * this the user has to construct and wire up an `Observability` themselves. If the package is + * absent, the integration keeps its Node `createRequire` fallback and the marker stays empty. */ -export function sentryMastraObservabilityProviderPlugin(): { - name: string; - configResolved(config: { root: string }): void; - transform(code: string, id: string): { code: string; map: ReturnType } | undefined; -} { - let providerSnippet: string | undefined; - - return { +export function sentryMastraObservabilityProviderPlugin(): ProvidedModulePlugin { + return createProvidedModulePlugin({ name: 'sentry-cloudflare-mastra-observability-provider', - - configResolved(config: { root: string }): void { - // Resolved at build time (Node), so this `createRequire` never ships to the worker. - try { - createRequire(resolve(config.root, 'noop.js')).resolve('@mastra/observability'); - } catch { - return; - } - providerSnippet = - `import * as ${PROVIDER_IDENTIFIER} from '@mastra/observability';\n` + - '(globalThis.__SENTRY_ORCHESTRION__ = globalThis.__SENTRY_ORCHESTRION__ || {});\n' + - '(globalThis.__SENTRY_ORCHESTRION__.providedModules = globalThis.__SENTRY_ORCHESTRION__.providedModules || {})' + - `['@mastra/observability'] = ${PROVIDER_IDENTIFIER};\n`; - }, - - transform(code: string, id: string): { code: string; map: ReturnType } | undefined { - if (!providerSnippet || !isMastraIntegrationModuleId(id)) return undefined; - - const ms = new MagicString(code); - ms.prepend(providerSnippet); - return { code: ms.toString(), map: ms.generateMap({ hires: true }) }; - }, - }; + moduleName: '@mastra/observability', + identifier: '__SENTRY_MASTRA_OBSERVABILITY__', + integrationModule: 'mastra', + }); } diff --git a/packages/cloudflare/src/vite/providedModulePlugin.ts b/packages/cloudflare/src/vite/providedModulePlugin.ts new file mode 100644 index 000000000000..ca4e0acfdd6d --- /dev/null +++ b/packages/cloudflare/src/vite/providedModulePlugin.ts @@ -0,0 +1,114 @@ +import { resolve } from 'node:path'; +import { escapeStringForRegex } from '@sentry/core'; +import MagicString from 'magic-string'; + +/** + * The slice of the Rollup plugin context the probe needs. Declared here rather than imported so + * this file carries no Rollup or Vite type dependency. + */ +interface ResolveContext { + resolve( + source: string, + importer?: string, + options?: { skipSelf?: boolean }, + ): Promise<{ id: string; external?: boolean | string } | null>; +} + +/** The plugin shape `sentryCloudflareVitePlugin` composes. */ +export interface ProvidedModulePlugin { + name: string; + configResolved(config: { root: string }): void; + buildStart(this: ResolveContext): Promise; + transform(code: string, id: string): { code: string; map: ReturnType } | undefined; +} + +export interface ProvidedModulePluginOptions { + /** Vite plugin name, e.g. `sentry-cloudflare-flue-runtime-provider`. */ + name: string; + /** Bare specifier of the package to provide, e.g. `@flue/runtime`. */ + moduleName: string; + /** Namespace binding the injected import uses, e.g. `__SENTRY_FLUE_RUNTIME__`. */ + identifier: string; + /** Basename of the `@sentry/server-utils` integration module to inject into, e.g. `flue`. */ + integrationModule: string; +} + +/** Build the matcher for one `@sentry/server-utils` integration module. */ +export function createIntegrationModuleMatcher(integrationModule: string): (id: string) => boolean { + // The ESM build only: a worker never loads the CJS one. + const pattern = new RegExp( + `@sentry/server-utils/build/esm/integrations/${escapeStringForRegex(integrationModule)}\\.js$`, + ); + + return (id: string): boolean => pattern.test(id.replace(/\\/g, '/').replace(/[?#].*$/, '')); +} + +function buildProviderSnippet({ moduleName, identifier }: ProvidedModulePluginOptions): string { + const marker = 'globalThis.__SENTRY_ORCHESTRION__'; + + // A getter, not an assignment: assigning reads the binding at injection time, so it stores + // `undefined` whenever the bundler evaluates Sentry's module before the provided package + // finished initializing. Enumerable so the entry shows up in `Object.keys` and a spread. + return ( + `import * as ${identifier} from '${moduleName}';\n` + + `(${marker} = ${marker} || {});\n` + + `(${marker}.providedModules = ${marker}.providedModules || {});\n` + + `Object.defineProperty(${marker}.providedModules, '${moduleName}', ` + + `{ configurable: true, enumerable: true, get() { return ${identifier}; } });\n` + ); +} + +/** + * Build a Vite plugin that splices a static `import * as … from ''` into one of + * Sentry's own integration modules and exposes the namespace on the global orchestrion marker. + * + * Some packages are instrumented by registration rather than by patching, so instrumenting them + * needs a reference to that module's own binding and no channel payload carries one. On Node the + * integration resolves it itself; a bundled worker has no `node_modules` to resolve from, so the + * binding is supplied at build time instead. The import is static, lands in Sentry's module rather + * than the user's code, and is only emitted when the package actually resolves. + */ +export function createProvidedModulePlugin(options: ProvidedModulePluginOptions): ProvidedModulePlugin { + const isIntegrationModuleId = createIntegrationModuleMatcher(options.integrationModule); + + let root = process.cwd(); + let providerSnippet: string | undefined; + + return { + name: options.name, + + configResolved(config: { root: string }): void { + root = config.root; + }, + + async buildStart(this: ResolveContext): Promise { + // Already answered by an earlier environment. Resolution is per environment, and only the + // worker one ever reaches `transform`, so the first package found stands for the build. + if (providerSnippet) return; + + try { + // The environment's own resolver, so the probe uses the conditions the injected import + // will. That is what a `require.resolve` probe cannot do: an ESM-only package has no + // `require` condition and reads as missing. Resolved from the app root, not from Sentry's + // own install. + const resolved = await this.resolve(options.moduleName, resolve(root, 'noop.js')); + if (!resolved) return; + } catch { + // Installed but unresolvable for some other reason. Inject anyway so the build reports it, + // rather than silently shipping a worker with no instrumentation. + } + + providerSnippet = buildProviderSnippet(options); + }, + + transform(code: string, id: string): { code: string; map: ReturnType } | undefined { + // `code.includes` keeps this idempotent: a second pass over already-injected output would + // otherwise emit a duplicate `import * as` binding, which is a syntax error. + if (!providerSnippet || !isIntegrationModuleId(id) || code.includes(options.identifier)) return undefined; + + const ms = new MagicString(code); + ms.prepend(providerSnippet); + return { code: ms.toString(), map: ms.generateMap({ hires: true }) }; + }, + }; +} diff --git a/packages/cloudflare/test/vite/flueRuntime.test.ts b/packages/cloudflare/test/vite/flueRuntime.test.ts index 8ebc1ae595f0..8279a5a74106 100644 --- a/packages/cloudflare/test/vite/flueRuntime.test.ts +++ b/packages/cloudflare/test/vite/flueRuntime.test.ts @@ -1,153 +1,32 @@ -import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { beforeAll, describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; +import { sentryFlueRuntimeProviderPlugin } from '../../src/vite/flueRuntime'; import { sentryCloudflareVitePlugin } from '../../src/vite/index'; -import { isFlueIntegrationModuleId, sentryFlueRuntimeProviderPlugin } from '../../src/vite/flueRuntime'; const PROVIDER_PLUGIN = 'sentry-cloudflare-flue-runtime-provider'; const FLUE_INTEGRATION_MODULE = '/app/node_modules/@sentry/server-utils/build/esm/integrations/flue.js'; -/** An app root whose `node_modules` holds an ESM-only `@flue/runtime`, as published. */ -function createRootWithFlue(): string { - const root = mkdtempSync(join(tmpdir(), 'sentry-flue-root-')); - const pkgDir = join(root, 'node_modules', '@flue', 'runtime'); - mkdirSync(join(pkgDir, 'dist'), { recursive: true }); - writeFileSync( - join(pkgDir, 'package.json'), - // No `require` condition — the reason `resolve()` reports ERR_PACKAGE_PATH_NOT_EXPORTED. - JSON.stringify({ - name: '@flue/runtime', - version: '2.0.8', - type: 'module', - exports: { '.': { import: './dist/index.mjs' } }, - }), - ); - writeFileSync(join(pkgDir, 'dist', 'index.mjs'), 'export const instrument = () => {};\n'); - return root; -} - -function createEmptyRoot(): string { - return mkdtempSync(join(tmpdir(), 'sentry-flue-empty-')); -} - -/** An app root holding an installed but unreadable `@flue/runtime`. */ -function createRootWithBrokenFlue(): string { - const root = mkdtempSync(join(tmpdir(), 'sentry-flue-broken-')); - const pkgDir = join(root, 'node_modules', '@flue', 'runtime'); - mkdirSync(pkgDir, { recursive: true }); - writeFileSync(join(pkgDir, 'package.json'), '{ not json'); - return root; -} - -describe('isFlueIntegrationModuleId', () => { - it('matches the ESM Flue integration module', () => { - expect(isFlueIntegrationModuleId(FLUE_INTEGRATION_MODULE)).toBe(true); - }); - - it('ignores a trailing query/hash Vite may append', () => { - expect(isFlueIntegrationModuleId(`${FLUE_INTEGRATION_MODULE}?v=abc`)).toBe(true); - }); - - it('normalizes Windows separators', () => { - expect( - isFlueIntegrationModuleId('C:\\app\\node_modules\\@sentry\\server-utils\\build\\esm\\integrations\\flue.js'), - ).toBe(true); - }); - - it('does not match the CJS build (workers load ESM)', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/cjs/integrations/flue.js')).toBe( - false, - ); - }); - - it('does not match another integration module', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js')).toBe( - false, - ); - }); - - it('does not match Flue itself', () => { - expect(isFlueIntegrationModuleId('/app/node_modules/@flue/runtime/dist/index.mjs')).toBe(false); - }); -}); - describe('sentryFlueRuntimeProviderPlugin', () => { - describe('when the app has @flue/runtime installed', () => { - let root: string; - - beforeAll(() => { - root = createRootWithFlue(); - }); - - it('injects the provider even though the package is ESM-only', () => { - // Regression guard: treating that error as "absent" silently disabled auto-instrumentation. - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root }); - - const result = plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE); - - expect(result?.code).toContain("import * as __SENTRY_FLUE_RUNTIME__ from '@flue/runtime';"); - expect(result?.code).toContain('__SENTRY_ORCHESTRION__.providedModules'); - expect(result?.code).toContain('export const x = 1;'); - }); - - it('exposes the namespace through a getter rather than a snapshot', () => { - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root }); - - expect(plugin.transform('', FLUE_INTEGRATION_MODULE)?.code).toContain( - 'get() { return __SENTRY_FLUE_RUNTIME__; }', - ); - }); - - it('leaves every other module untouched', () => { - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root }); + it('injects `@flue/runtime` behind a getter', async () => { + // A getter, not an assignment: the bundler may evaluate Sentry's module before + // `@flue/runtime` is initialized, and assigning there would store `undefined`. + const plugin = sentryFlueRuntimeProviderPlugin(); + plugin.configResolved({ root: '/app' }); + const resolve = vi.fn(async () => ({ id: '/app/node_modules/@flue/runtime/dist/index.mjs' })); + await plugin.buildStart.call({ resolve }); - expect(plugin.transform('export const x = 1;', '/app/src/index.ts')).toBeUndefined(); - }); + const code = plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)?.code; - it('injects once, so a second pass cannot emit a duplicate binding', () => { - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root }); - - const once = plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)?.code ?? ''; - - expect(plugin.transform(once, FLUE_INTEGRATION_MODULE)).toBeUndefined(); - }); - }); - - describe('when @flue/runtime is installed but unresolvable', () => { - it('still injects, so the failure surfaces from Vite instead of silently disabling tracing', () => { - // Only a module-not-found means absent. Skipping on every other resolve failure is how an - // installed package silently loses instrumentation, which is the bug this plugin fixes. - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root: createRootWithBrokenFlue() }); - - expect(plugin.transform('', FLUE_INTEGRATION_MODULE)).toBeDefined(); - }); + expect(resolve).toHaveBeenCalledWith('@flue/runtime', '/app/noop.js'); + expect(code).toContain("import * as __SENTRY_FLUE_RUNTIME__ from '@flue/runtime';"); + expect(code).toContain('get() { return __SENTRY_FLUE_RUNTIME__; }'); }); - describe('when the app does not have @flue/runtime installed', () => { - it('injects nothing', () => { - const plugin = sentryFlueRuntimeProviderPlugin(); - plugin.configResolved({ root: createEmptyRoot() }); - - expect(plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)).toBeUndefined(); - }); - - it("resolves from the app root, not from Sentry's own install", () => { - // This repo has no `@flue/runtime`, so only an app root that does can pass the check. - const withFlue = sentryFlueRuntimeProviderPlugin(); - withFlue.configResolved({ root: createRootWithFlue() }); - - const withoutFlue = sentryFlueRuntimeProviderPlugin(); - withoutFlue.configResolved({ root: createEmptyRoot() }); + it('injects nothing when the app has no @flue/runtime', async () => { + const plugin = sentryFlueRuntimeProviderPlugin(); + plugin.configResolved({ root: '/app' }); + await plugin.buildStart.call({ resolve: vi.fn(async () => null) }); - expect(withFlue.transform('', FLUE_INTEGRATION_MODULE)).toBeDefined(); - expect(withoutFlue.transform('', FLUE_INTEGRATION_MODULE)).toBeUndefined(); - }); + expect(plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)).toBeUndefined(); }); }); diff --git a/packages/cloudflare/test/vite/mastraObservability.test.ts b/packages/cloudflare/test/vite/mastraObservability.test.ts index fd2d810ebef1..0862073e4eb9 100644 --- a/packages/cloudflare/test/vite/mastraObservability.test.ts +++ b/packages/cloudflare/test/vite/mastraObservability.test.ts @@ -1,42 +1,43 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { sentryCloudflareVitePlugin } from '../../src/vite/index'; -import { isMastraIntegrationModuleId } from '../../src/vite/mastraObservability'; +import { sentryMastraObservabilityProviderPlugin } from '../../src/vite/mastraObservability'; const PROVIDER_PLUGIN = 'sentry-cloudflare-mastra-observability-provider'; -describe('isMastraIntegrationModuleId', () => { - it('matches the ESM Mastra integration module', () => { - expect(isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js')).toBe( - true, - ); - }); +describe('sentryMastraObservabilityProviderPlugin', () => { + const MASTRA_INTEGRATION_MODULE = '/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js'; - it('ignores a trailing query/hash Vite may append', () => { - expect( - isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js?v=abc'), - ).toBe(true); - }); + it('exposes `@mastra/observability` on the marker behind a getter', async () => { + const plugin = sentryMastraObservabilityProviderPlugin(); + plugin.configResolved({ root: '/app' }); + const resolve = vi.fn(async () => ({ id: '/app/node_modules/@mastra/observability/dist/index.js' })); + await plugin.buildStart.call({ resolve }); - it('normalizes Windows separators', () => { - expect( - isMastraIntegrationModuleId('C:\\app\\node_modules\\@sentry\\server-utils\\build\\esm\\integrations\\mastra.js'), - ).toBe(true); - }); + const code = plugin.transform('export const x = 1;', MASTRA_INTEGRATION_MODULE)?.code; - it('does not match the CJS build (workers load ESM)', () => { - expect(isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/cjs/integrations/mastra.js')).toBe( - false, - ); + expect(resolve).toHaveBeenCalledWith('@mastra/observability', '/app/noop.js'); + expect(code).toContain("import * as __SENTRY_MASTRA_OBSERVABILITY__ from '@mastra/observability';"); + expect(code).toContain('get() { return __SENTRY_MASTRA_OBSERVABILITY__; }'); }); - it('does not match another integration module', () => { - expect(isMastraIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/openai.js')).toBe( - false, - ); + it('injects nothing when the app has no @mastra/observability', async () => { + const plugin = sentryMastraObservabilityProviderPlugin(); + plugin.configResolved({ root: '/app' }); + await plugin.buildStart.call({ resolve: vi.fn(async () => null) }); + + expect(plugin.transform('export const x = 1;', MASTRA_INTEGRATION_MODULE)).toBeUndefined(); }); - it('does not match unrelated modules', () => { - expect(isMastraIntegrationModuleId('/app/node_modules/@mastra/core/dist/index.js')).toBe(false); + it('keeps injecting for an ESM-only release', async () => { + // The old `createRequire().resolve()` probe read an ESM-only package as missing, because it + // has no `require` condition. `this.resolve()` uses the environment's own conditions. + const plugin = sentryMastraObservabilityProviderPlugin(); + plugin.configResolved({ root: '/app' }); + await plugin.buildStart.call({ + resolve: vi.fn(async () => ({ id: '/app/node_modules/@mastra/observability/dist/index.js' })), + }); + + expect(plugin.transform('', MASTRA_INTEGRATION_MODULE)).toBeDefined(); }); }); diff --git a/packages/cloudflare/test/vite/providedModulePlugin.test.ts b/packages/cloudflare/test/vite/providedModulePlugin.test.ts new file mode 100644 index 000000000000..48ecec523ca1 --- /dev/null +++ b/packages/cloudflare/test/vite/providedModulePlugin.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it, vi } from 'vitest'; +import type { ProvidedModulePluginOptions } from '../../src/vite/providedModulePlugin'; +import { createIntegrationModuleMatcher, createProvidedModulePlugin } from '../../src/vite/providedModulePlugin'; + +const TARGET = '/app/node_modules/@sentry/server-utils/build/esm/integrations/flue.js'; + +const OPTIONS: ProvidedModulePluginOptions = { + name: 'sentry-test-provider', + moduleName: '@scope/pkg', + identifier: '__SENTRY_TEST_PKG__', + integrationModule: 'flue', +}; + +/** A Rollup plugin context whose `resolve` answers however the test wants. */ +function pluginContext(resolve: (source: string, importer?: string) => unknown): { + resolve: ReturnType; +} { + return { resolve: vi.fn(async (source: string, importer?: string) => resolve(source, importer)) }; +} + +const found = pluginContext(() => ({ id: '/app/node_modules/@scope/pkg/dist/index.mjs' })); +const missing = pluginContext(() => null); + +/** Run `configResolved` + `buildStart` the way Vite would, then hand the plugin back. */ +async function start( + options: Partial, + context: { resolve: ReturnType }, + root = '/app', +): Promise> { + const plugin = createProvidedModulePlugin({ ...OPTIONS, ...options }); + plugin.configResolved({ root }); + await plugin.buildStart.call(context); + return plugin; +} + +describe('createIntegrationModuleMatcher', () => { + const isFlueIntegrationModuleId = createIntegrationModuleMatcher('flue'); + + it('matches the ESM integration module', () => { + expect(isFlueIntegrationModuleId(TARGET)).toBe(true); + }); + + it('ignores a trailing query/hash Vite may append', () => { + expect(isFlueIntegrationModuleId(`${TARGET}?v=abc`)).toBe(true); + }); + + it('normalizes Windows separators', () => { + expect( + isFlueIntegrationModuleId('C:\\app\\node_modules\\@sentry\\server-utils\\build\\esm\\integrations\\flue.js'), + ).toBe(true); + }); + + it('does not match the CJS build (workers load ESM)', () => { + expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/cjs/integrations/flue.js')).toBe( + false, + ); + }); + + it('does not match another integration module', () => { + expect(isFlueIntegrationModuleId('/app/node_modules/@sentry/server-utils/build/esm/integrations/mastra.js')).toBe( + false, + ); + }); + + it('does not match the instrumented package itself', () => { + expect(isFlueIntegrationModuleId('/app/node_modules/@flue/runtime/dist/index.mjs')).toBe(false); + expect(createIntegrationModuleMatcher('mastra')('/app/node_modules/@mastra/core/dist/index.js')).toBe(false); + }); +}); + +describe('createProvidedModulePlugin', () => { + it('injects the import and the marker when the package resolves', async () => { + const plugin = await start( + {}, + pluginContext(() => ({ id: '/x' })), + ); + + const result = plugin.transform('export const x = 1;', TARGET); + + expect(result?.code).toContain("import * as __SENTRY_TEST_PKG__ from '@scope/pkg';"); + expect(result?.code).toContain('__SENTRY_ORCHESTRION__.providedModules'); + expect(result?.code).toContain('export const x = 1;'); + }); + + it('injects nothing when the package does not resolve', async () => { + const plugin = await start({}, missing); + + expect(plugin.transform('export const x = 1;', TARGET)).toBeUndefined(); + }); + + it('still injects when resolution throws, so the build reports it', async () => { + // Skipping on a resolver error is how an installed package silently loses instrumentation. + const plugin = await start( + {}, + pluginContext(() => { + throw new Error('invalid package.json'); + }), + ); + + expect(plugin.transform('', TARGET)).toBeDefined(); + }); + + it('probes the package from the app root', async () => { + const context = pluginContext(() => ({ id: '/x' })); + await start({}, context, '/srv/my-worker'); + + expect(context.resolve).toHaveBeenCalledWith('@scope/pkg', '/srv/my-worker/noop.js'); + }); + + it('exposes the namespace through an enumerable getter, never an assignment', async () => { + // Assignment reads the binding at injection time, so it stores `undefined` whenever the + // bundler evaluates Sentry's module first. + const plugin = await start({}, found); + + const code = plugin.transform('', TARGET)?.code; + + expect(code).toContain('enumerable: true'); + expect(code).toContain('get() { return __SENTRY_TEST_PKG__; }'); + expect(code).not.toContain("providedModules['@scope/pkg'] ="); + }); + + it('leaves every other module untouched', async () => { + const plugin = await start({}, found); + + expect(plugin.transform('export const x = 1;', '/app/src/index.ts')).toBeUndefined(); + }); + + it('injects once, so a second pass cannot emit a duplicate binding', async () => { + const plugin = await start({}, found); + + const once = plugin.transform('export const x = 1;', TARGET)?.code ?? ''; + + expect(plugin.transform(once, TARGET)).toBeUndefined(); + }); + + it('stops probing once the package is found', async () => { + // Vite runs `buildStart` per environment against a shared plugin instance. + const context = pluginContext(() => ({ id: '/x' })); + const plugin = await start({}, context); + await plugin.buildStart.call(context); + + expect(context.resolve).toHaveBeenCalledTimes(1); + }); + + it('probes again in the next environment when the first cannot resolve', async () => { + // Only the worker environment resolves the worker's dependencies, and it may not run first. + let resolvable = false; + const context = pluginContext(() => (resolvable ? { id: '/x' } : null)); + const plugin = await start({}, context); + + resolvable = true; + await plugin.buildStart.call(context); + + expect(context.resolve).toHaveBeenCalledTimes(2); + expect(plugin.transform('', TARGET)).toBeDefined(); + }); +}); From 73b753b24ff6ff9966d6a6bfa0adcd09f092f521 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 21 Sep 2026 17:45:27 +0300 Subject: [PATCH 2/4] fix(cloudflare): Probe for provided modules in server environments only Two defects in the factory, both found reviewing it against real Vite 6.4.3. `buildStart` runs once per environment against one shared plugin instance, and nothing gated which environments it applied to. A two-environment build proved the `client` environment probes first, resolves under browser conditions, and short-circuits the worker so its resolver is never consulted. That defeats the reason the probe moved off `createRequire` at all. `applyToEnvironment` now gates to server consumers, matching the orchestrion plugin. The matcher built a `RegExp` from a caller-supplied string, which needed `escapeStringForRegex` and so put the first `@sentry/core` import into `src/vite/`. Every other module there stays on `node:*`, `magic-string` and `wrangler`, and the orchestrion config module documents the same rule: a build-time plugin must not drag the SDK into the build. The pattern was end-anchored with nothing else to match, so `endsWith` replaces it exactly and the import is gone. Co-Authored-By: Claude Opus 5 --- .../src/vite/providedModulePlugin.ts | 32 ++++++++++++++----- .../test/vite/providedModulePlugin.test.ts | 9 ++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/packages/cloudflare/src/vite/providedModulePlugin.ts b/packages/cloudflare/src/vite/providedModulePlugin.ts index ca4e0acfdd6d..b5cf1a38da8e 100644 --- a/packages/cloudflare/src/vite/providedModulePlugin.ts +++ b/packages/cloudflare/src/vite/providedModulePlugin.ts @@ -1,5 +1,4 @@ import { resolve } from 'node:path'; -import { escapeStringForRegex } from '@sentry/core'; import MagicString from 'magic-string'; /** @@ -17,6 +16,7 @@ interface ResolveContext { /** The plugin shape `sentryCloudflareVitePlugin` composes. */ export interface ProvidedModulePlugin { name: string; + applyToEnvironment(environment: { config: { consumer: string } }): boolean; configResolved(config: { root: string }): void; buildStart(this: ResolveContext): Promise; transform(code: string, id: string): { code: string; map: ReturnType } | undefined; @@ -33,14 +33,22 @@ export interface ProvidedModulePluginOptions { integrationModule: string; } -/** Build the matcher for one `@sentry/server-utils` integration module. */ +/** + * Build the matcher for one `@sentry/server-utils` integration module. + * + * Plain `endsWith`, not a `RegExp`: nothing here needs pattern matching, and building one from + * a caller-supplied string would need escaping, which is the only reason this file would have to + * import from `@sentry/core`. A build-time plugin should not drag the SDK into the build. + */ export function createIntegrationModuleMatcher(integrationModule: string): (id: string) => boolean { // The ESM build only: a worker never loads the CJS one. - const pattern = new RegExp( - `@sentry/server-utils/build/esm/integrations/${escapeStringForRegex(integrationModule)}\\.js$`, - ); + const suffix = `@sentry/server-utils/build/esm/integrations/${integrationModule}.js`; - return (id: string): boolean => pattern.test(id.replace(/\\/g, '/').replace(/[?#].*$/, '')); + return (id: string): boolean => + id + .replace(/\\/g, '/') + .replace(/[?#].*$/, '') + .endsWith(suffix); } function buildProviderSnippet({ moduleName, identifier }: ProvidedModulePluginOptions): string { @@ -77,13 +85,21 @@ export function createProvidedModulePlugin(options: ProvidedModulePluginOptions) return { name: options.name, + applyToEnvironment(environment: { config: { consumer: string } }): boolean { + // Server environments only. `buildStart` runs per environment against one shared plugin + // instance, so without this a `client` build resolves first, under browser conditions, and + // answers on the worker's behalf. That defeats the point of probing with `this.resolve`. + // Same gate the orchestrion plugin uses. + return environment.config.consumer === 'server'; + }, + configResolved(config: { root: string }): void { root = config.root; }, async buildStart(this: ResolveContext): Promise { - // Already answered by an earlier environment. Resolution is per environment, and only the - // worker one ever reaches `transform`, so the first package found stands for the build. + // Already answered by an earlier server environment. A build with several worker + // environments shares the answer: they resolve under the same conditions. if (providerSnippet) return; try { diff --git a/packages/cloudflare/test/vite/providedModulePlugin.test.ts b/packages/cloudflare/test/vite/providedModulePlugin.test.ts index 48ecec523ca1..1726292d93d2 100644 --- a/packages/cloudflare/test/vite/providedModulePlugin.test.ts +++ b/packages/cloudflare/test/vite/providedModulePlugin.test.ts @@ -82,6 +82,15 @@ describe('createProvidedModulePlugin', () => { expect(result?.code).toContain('export const x = 1;'); }); + it('runs in server environments only', () => { + // `buildStart` runs per environment against one shared instance. A `client` build resolves + // under browser conditions, so letting it probe answers on the worker's behalf. + const plugin = createProvidedModulePlugin(OPTIONS); + + expect(plugin.applyToEnvironment({ config: { consumer: 'server' } })).toBe(true); + expect(plugin.applyToEnvironment({ config: { consumer: 'client' } })).toBe(false); + }); + it('injects nothing when the package does not resolve', async () => { const plugin = await start({}, missing); From aa361fae7c8e52dca3cf9e33810b90ff55ddc99f Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 21 Sep 2026 18:19:22 +0300 Subject: [PATCH 3/4] fix(cloudflare): Report why a provided-module probe failed The `catch` around `this.resolve` swallowed the error and injected anyway. The injection is the right call, since skipping is how an installed package silently loses instrumentation, but the import error Vite raises next says nothing about why resolution broke. Warn with the original cause. The Flue and Mastra suites built plugin contexts with only `resolve`, which would have thrown on the new `this.warn` call had those tests taken the catch path. `strictBindCallApply` does not catch this through `Function.call`, so they typechecked clean while carrying the trap. Co-Authored-By: Claude Opus 5 --- .../src/vite/providedModulePlugin.ts | 13 ++++++++--- .../cloudflare/test/vite/flueRuntime.test.ts | 4 ++-- .../test/vite/mastraObservability.test.ts | 5 +++-- .../test/vite/providedModulePlugin.test.ts | 22 +++++++++++-------- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/packages/cloudflare/src/vite/providedModulePlugin.ts b/packages/cloudflare/src/vite/providedModulePlugin.ts index b5cf1a38da8e..1c63bd226741 100644 --- a/packages/cloudflare/src/vite/providedModulePlugin.ts +++ b/packages/cloudflare/src/vite/providedModulePlugin.ts @@ -11,6 +11,7 @@ interface ResolveContext { importer?: string, options?: { skipSelf?: boolean }, ): Promise<{ id: string; external?: boolean | string } | null>; + warn(message: string): void; } /** The plugin shape `sentryCloudflareVitePlugin` composes. */ @@ -109,9 +110,15 @@ export function createProvidedModulePlugin(options: ProvidedModulePluginOptions) // own install. const resolved = await this.resolve(options.moduleName, resolve(root, 'noop.js')); if (!resolved) return; - } catch { - // Installed but unresolvable for some other reason. Inject anyway so the build reports it, - // rather than silently shipping a worker with no instrumentation. + } catch (error) { + // Present but unresolvable for some other reason. Inject anyway so the build fails loudly + // rather than silently shipping a worker with no instrumentation, and surface the original + // cause: the import error Vite raises next says nothing about why resolution broke. + this.warn( + `[Sentry] could not resolve ${options.moduleName} while probing for it; injecting the provider anyway. ${ + (error as Error | undefined)?.message ?? error + }`, + ); } providerSnippet = buildProviderSnippet(options); diff --git a/packages/cloudflare/test/vite/flueRuntime.test.ts b/packages/cloudflare/test/vite/flueRuntime.test.ts index 8279a5a74106..5b9d1d36d429 100644 --- a/packages/cloudflare/test/vite/flueRuntime.test.ts +++ b/packages/cloudflare/test/vite/flueRuntime.test.ts @@ -12,7 +12,7 @@ describe('sentryFlueRuntimeProviderPlugin', () => { const plugin = sentryFlueRuntimeProviderPlugin(); plugin.configResolved({ root: '/app' }); const resolve = vi.fn(async () => ({ id: '/app/node_modules/@flue/runtime/dist/index.mjs' })); - await plugin.buildStart.call({ resolve }); + await plugin.buildStart.call({ resolve, warn: vi.fn() }); const code = plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)?.code; @@ -24,7 +24,7 @@ describe('sentryFlueRuntimeProviderPlugin', () => { it('injects nothing when the app has no @flue/runtime', async () => { const plugin = sentryFlueRuntimeProviderPlugin(); plugin.configResolved({ root: '/app' }); - await plugin.buildStart.call({ resolve: vi.fn(async () => null) }); + await plugin.buildStart.call({ resolve: vi.fn(async () => null), warn: vi.fn() }); expect(plugin.transform('export const x = 1;', FLUE_INTEGRATION_MODULE)).toBeUndefined(); }); diff --git a/packages/cloudflare/test/vite/mastraObservability.test.ts b/packages/cloudflare/test/vite/mastraObservability.test.ts index 0862073e4eb9..08efb4b30c0b 100644 --- a/packages/cloudflare/test/vite/mastraObservability.test.ts +++ b/packages/cloudflare/test/vite/mastraObservability.test.ts @@ -11,7 +11,7 @@ describe('sentryMastraObservabilityProviderPlugin', () => { const plugin = sentryMastraObservabilityProviderPlugin(); plugin.configResolved({ root: '/app' }); const resolve = vi.fn(async () => ({ id: '/app/node_modules/@mastra/observability/dist/index.js' })); - await plugin.buildStart.call({ resolve }); + await plugin.buildStart.call({ resolve, warn: vi.fn() }); const code = plugin.transform('export const x = 1;', MASTRA_INTEGRATION_MODULE)?.code; @@ -23,7 +23,7 @@ describe('sentryMastraObservabilityProviderPlugin', () => { it('injects nothing when the app has no @mastra/observability', async () => { const plugin = sentryMastraObservabilityProviderPlugin(); plugin.configResolved({ root: '/app' }); - await plugin.buildStart.call({ resolve: vi.fn(async () => null) }); + await plugin.buildStart.call({ resolve: vi.fn(async () => null), warn: vi.fn() }); expect(plugin.transform('export const x = 1;', MASTRA_INTEGRATION_MODULE)).toBeUndefined(); }); @@ -35,6 +35,7 @@ describe('sentryMastraObservabilityProviderPlugin', () => { plugin.configResolved({ root: '/app' }); await plugin.buildStart.call({ resolve: vi.fn(async () => ({ id: '/app/node_modules/@mastra/observability/dist/index.js' })), + warn: vi.fn(), }); expect(plugin.transform('', MASTRA_INTEGRATION_MODULE)).toBeDefined(); diff --git a/packages/cloudflare/test/vite/providedModulePlugin.test.ts b/packages/cloudflare/test/vite/providedModulePlugin.test.ts index 1726292d93d2..4ec94a089012 100644 --- a/packages/cloudflare/test/vite/providedModulePlugin.test.ts +++ b/packages/cloudflare/test/vite/providedModulePlugin.test.ts @@ -14,8 +14,12 @@ const OPTIONS: ProvidedModulePluginOptions = { /** A Rollup plugin context whose `resolve` answers however the test wants. */ function pluginContext(resolve: (source: string, importer?: string) => unknown): { resolve: ReturnType; + warn: ReturnType; } { - return { resolve: vi.fn(async (source: string, importer?: string) => resolve(source, importer)) }; + return { + resolve: vi.fn(async (source: string, importer?: string) => resolve(source, importer)), + warn: vi.fn(), + }; } const found = pluginContext(() => ({ id: '/app/node_modules/@scope/pkg/dist/index.mjs' })); @@ -24,7 +28,7 @@ const missing = pluginContext(() => null); /** Run `configResolved` + `buildStart` the way Vite would, then hand the plugin back. */ async function start( options: Partial, - context: { resolve: ReturnType }, + context: ReturnType, root = '/app', ): Promise> { const plugin = createProvidedModulePlugin({ ...OPTIONS, ...options }); @@ -97,16 +101,16 @@ describe('createProvidedModulePlugin', () => { expect(plugin.transform('export const x = 1;', TARGET)).toBeUndefined(); }); - it('still injects when resolution throws, so the build reports it', async () => { + it('still injects when resolution throws, and reports the cause', async () => { // Skipping on a resolver error is how an installed package silently loses instrumentation. - const plugin = await start( - {}, - pluginContext(() => { - throw new Error('invalid package.json'); - }), - ); + // The import error Vite raises next says nothing about why resolution broke, so warn with it. + const context = pluginContext(() => { + throw new Error('invalid package.json'); + }); + const plugin = await start({}, context); expect(plugin.transform('', TARGET)).toBeDefined(); + expect(context.warn).toHaveBeenCalledWith(expect.stringContaining('invalid package.json')); }); it('probes the package from the app root', async () => { From d7e1b10a1023952feb780268d3bd963f79fcb126 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 21 Sep 2026 18:22:56 +0300 Subject: [PATCH 4/4] test(cloudflare): Give each provider-plugin test its own mock context `found` and `missing` were module-scope `vi.fn()` instances shared by four tests, with no reset between them, so their call counts accumulated. Nothing asserts on those counts today, but the next test that tried would pass in isolation and fail in suite order. Co-Authored-By: Claude Opus 5 --- .../test/vite/providedModulePlugin.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/cloudflare/test/vite/providedModulePlugin.test.ts b/packages/cloudflare/test/vite/providedModulePlugin.test.ts index 4ec94a089012..5a138cde1a19 100644 --- a/packages/cloudflare/test/vite/providedModulePlugin.test.ts +++ b/packages/cloudflare/test/vite/providedModulePlugin.test.ts @@ -22,8 +22,9 @@ function pluginContext(resolve: (source: string, importer?: string) => unknown): }; } -const found = pluginContext(() => ({ id: '/app/node_modules/@scope/pkg/dist/index.mjs' })); -const missing = pluginContext(() => null); +const found = (): ReturnType => + pluginContext(() => ({ id: '/app/node_modules/@scope/pkg/dist/index.mjs' })); +const missing = (): ReturnType => pluginContext(() => null); /** Run `configResolved` + `buildStart` the way Vite would, then hand the plugin back. */ async function start( @@ -96,7 +97,7 @@ describe('createProvidedModulePlugin', () => { }); it('injects nothing when the package does not resolve', async () => { - const plugin = await start({}, missing); + const plugin = await start({}, missing()); expect(plugin.transform('export const x = 1;', TARGET)).toBeUndefined(); }); @@ -123,7 +124,7 @@ describe('createProvidedModulePlugin', () => { it('exposes the namespace through an enumerable getter, never an assignment', async () => { // Assignment reads the binding at injection time, so it stores `undefined` whenever the // bundler evaluates Sentry's module first. - const plugin = await start({}, found); + const plugin = await start({}, found()); const code = plugin.transform('', TARGET)?.code; @@ -133,13 +134,13 @@ describe('createProvidedModulePlugin', () => { }); it('leaves every other module untouched', async () => { - const plugin = await start({}, found); + const plugin = await start({}, found()); expect(plugin.transform('export const x = 1;', '/app/src/index.ts')).toBeUndefined(); }); it('injects once, so a second pass cannot emit a duplicate binding', async () => { - const plugin = await start({}, found); + const plugin = await start({}, found()); const once = plugin.transform('export const x = 1;', TARGET)?.code ?? '';