diff --git a/dev-packages/cloudflare-integration-tests/runner.ts b/dev-packages/cloudflare-integration-tests/runner.ts index 252d5dc84031..a5685c088812 100644 --- a/dev-packages/cloudflare-integration-tests/runner.ts +++ b/dev-packages/cloudflare-integration-tests/runner.ts @@ -132,6 +132,8 @@ type Expected = Envelope | ((envelope: Envelope) => void); type StartResult = { completed(): Promise; + /** Every non-ignored envelope received so far, matched or not, for count assertions. */ + getReceivedEnvelopes(): Envelope[]; makeRequest( method: 'get' | 'post', path: string, @@ -211,6 +213,7 @@ export function createRunner(...paths: string[]) { }); const expectedEnvelopeCount = expectedEnvelopes.length; + const receivedEnvelopes: Envelope[] = []; let envelopeCount = 0; const envelopeWaiters: { expected: Expected; resolve: () => void; reject: (e: unknown) => void }[] = []; @@ -256,6 +259,8 @@ export function createRunner(...paths: string[]) { return; } + receivedEnvelopes.push(envelope); + // Check per-request waiters first (FIFO order) if (envelopeWaiters.length > 0) { const waiter = envelopeWaiters.shift()!; @@ -416,6 +421,9 @@ export function createRunner(...paths: string[]) { completed: async function (): Promise { return isComplete; }, + getReceivedEnvelopes: function (): Envelope[] { + return receivedEnvelopes; + }, makeRequest: async function ( method: 'get' | 'post', path: string, diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/index.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/index.ts index 55b813a8daac..7ae665db507b 100644 --- a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/index.ts +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/index.ts @@ -8,10 +8,11 @@ interface Env { // `Counter` is imported from another module (`./counter`) where it was already // manually wrapped with `instrumentDurableObjectWithSentry`, then re-exported // here. The auto-instrument transform runs over this entry and sees -// `export { Counter }`, but `Counter` is an imported binding — not a local class -// declaration — so it cannot (and must not) wrap it. The DO stays instrumented -// solely via the manual wrap in `./counter`, and the plain default export below -// is still auto-wrapped with `withSentry`. +// `export { Counter }`, but nothing in this file reveals that the binding is +// already wrapped, so it emits its wrapper behind a guard +// (`_INTERNAL_wrapUnlessInstrumented`) that returns the manual wrap unchanged +// instead of nesting. The plain default export below is still auto-wrapped +// with `withSentry`. export { Counter }; export default { diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/test.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/test.ts index d09b4ceeeda6..bfe302220126 100644 --- a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/durableobject-reexport-instrumented/test.ts @@ -43,12 +43,13 @@ function expectMainWorkerTransaction(transactionEvent: TransactionEvent): void { // `Counter` is manually wrapped with `instrumentDurableObjectWithSentry` in a // separate module (`./counter`), imported into the entry, and re-exported via a -// plain `export { Counter }`. Because `Counter` is an imported binding rather -// than a local class declaration, the transform cannot wrap it in the entry and -// must leave it alone — no double-wrap, no broken build. The DO stays -// instrumented via the manual wrap, so we still expect a storage-bearing DO -// transaction, alongside the auto-wrapped default export's child-less one. -it('leaves an imported, already-instrumented Durable Object untouched and still wraps the default export', async ({ +// plain `export { Counter }`. The transform sees only the imported binding, so it +// emits its wrapper behind `_INTERNAL_wrapUnlessInstrumented`, which recognizes +// the hand-wrapped class and hands it straight back. Without that guard the two +// wrappers nest and every storage call reports twice, so the exactly-two span +// assertion below is the real check. The DO stays instrumented via the manual +// wrap, alongside the auto-wrapped default export's child-less transaction. +it('does not double-instrument an imported, already-wrapped Durable Object and still wraps the default export', async ({ signal, }) => { const runner = createRunner(__dirname) diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/greeter.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/greeter.ts new file mode 100644 index 000000000000..78cb5873f593 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/greeter.ts @@ -0,0 +1,19 @@ +import * as Sentry from '@sentry/cloudflare'; +import { WorkerEntrypoint } from 'cloudflare:workers'; + +interface Env { + SENTRY_DSN: string; +} + +class GreeterImpl extends WorkerEntrypoint { + async fetch(): Promise { + return new Response('Hello from the entrypoint'); + } +} + +// Manually instrumented here, in a module separate from the worker entry, which +// only imports and re-exports the wrapped class. +export const GreeterEntrypoint = Sentry.withSentry( + (env: Env) => ({ dsn: env.SENTRY_DSN, traceLifecycle: 'static', tracesSampleRate: 1.0 }), + GreeterImpl, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/index.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/index.ts new file mode 100644 index 000000000000..8199dac94312 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/index.ts @@ -0,0 +1,25 @@ +import { GreeterEntrypoint } from './greeter'; + +interface Env { + SENTRY_DSN: string; + SELF: Fetcher; +} + +// `GreeterEntrypoint` was already wrapped by hand in `./greeter`. The +// auto-instrument transform cannot see that from this entry (it only knows the +// class from the self service binding in wrangler.jsonc), so it emits its +// wrapper behind `_INTERNAL_wrapUnlessInstrumented`, which hands the manual +// wrap back unchanged instead of nesting a second wrapper around it. +export { GreeterEntrypoint }; + +export default { + async fetch(request: Request, env: Env): Promise { + const url = new URL(request.url); + + if (url.pathname === '/call-entrypoint') { + return env.SELF.fetch(new Request('https://self/greet')); + } + + return new Response('Not found', { status: 404 }); + }, +} satisfies ExportedHandler; diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/instrument.server.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/instrument.server.ts new file mode 100644 index 000000000000..4355b90010d6 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/instrument.server.ts @@ -0,0 +1,7 @@ +import { defineCloudflareOptions } from '@sentry/cloudflare'; + +export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({ + dsn: env.SENTRY_DSN, + traceLifecycle: 'static', + tracesSampleRate: 1.0, +})); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/test.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/test.ts new file mode 100644 index 000000000000..fe8f168b8e4c --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/test.ts @@ -0,0 +1,30 @@ +import type { TransactionEvent } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { createRunner } from '../../../runner'; + +// `GreeterEntrypoint` is hand-wrapped in `./greeter` and only re-exported by +// the entry, so the transform's emitted `_INTERNAL_wrapUnlessInstrumented` +// guard must hand the manual wrap back instead of nesting a second wrapper. +// Nested entrypoint wrappers each instrument `fetch`, which shows up as extra +// spans on the entrypoint transaction, the strict shape below catches that. +it('does not double-instrument an imported, already-wrapped WorkerEntrypoint', async ({ signal }) => { + const runner = createRunner(__dirname) + .unordered() + .expect(envelope => { + const transactionEvent = envelope[1]?.[0]?.[1] as TransactionEvent; + // The entrypoint's own transaction, child-less when wrapped exactly once. + expect(transactionEvent.transaction).toBe('GET /greet'); + expect(transactionEvent.contexts?.trace?.op).toBe('http.server'); + expect(transactionEvent.spans ?? []).toHaveLength(0); + }) + .expect(envelope => { + const transactionEvent = envelope[1]?.[0]?.[1] as TransactionEvent; + // The auto-wrapped default export's transaction. + expect(transactionEvent.transaction).toBe('GET /call-entrypoint'); + expect(transactionEvent.contexts?.trace?.op).toBe('http.server'); + }) + .start(signal); + + await runner.makeRequest('get', '/call-entrypoint'); + await runner.completed(); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/vite.config.mts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/vite.config.mts new file mode 100644 index 000000000000..f68e50f0019d --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/vite.config.mts @@ -0,0 +1,17 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + // The Sentry plugin runs first so its build-time transform wraps the worker + // entry and the self-bound `GreeterEntrypoint` before the Cloudflare plugin + // bundles it. + plugins: [ + cloudflare(), + sentryCloudflareVitePlugin({ + _experimental: { + autoInstrumentation: true, + }, + }), + ], +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/wrangler.jsonc new file mode 100644 index 000000000000..f11dd43e1730 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workerentrypoint-reexport-instrumented/wrangler.jsonc @@ -0,0 +1,18 @@ +{ + "$schema": "../../../node_modules/wrangler/config-schema.json", + "name": "cloudflare-vite-autoinstrument-workerentrypoint-reexport", + // `main` points at the source entry; the Sentry Vite plugin builds from it (so + // the auto-instrument transform runs) and the runner serves the built output. + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], + // Self-service-binding: names the entrypoint class, which is how the + // transform knows to wrap the re-exported binding at all. + "services": [ + { + "binding": "SELF", + "service": "cloudflare-vite-autoinstrument-workerentrypoint-reexport", + "entrypoint": "GreeterEntrypoint", + }, + ], +} diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/index.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/index.ts new file mode 100644 index 000000000000..080a6d9619ff --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/index.ts @@ -0,0 +1,46 @@ +import { MyWorkflow } from './workflow'; + +interface Env { + SENTRY_DSN: string; + MY_WORKFLOW: Workflow; +} + +// `MyWorkflow` was already wrapped by hand in `./workflow`. The auto-instrument +// transform cannot see that from this entry, so it emits its wrapper behind +// `_INTERNAL_wrapUnlessInstrumented`, which hands the manual wrap back unchanged +// instead of nesting a second wrapper around it. +export { MyWorkflow }; + +export default { + async fetch(request: Request, env: Env): Promise { + const url = new URL(request.url); + + // Issued by the test after `/trigger` returned, its transaction is the + // sentinel proving every earlier envelope (including a duplicate step + // transaction from an accidental double wrap) has been delivered. + if (url.pathname === '/sentinel') { + return new Response('ok'); + } + + if (url.pathname === '/trigger') { + const instance = await env.MY_WORKFLOW.create(); + // Respond only once the workflow finished, so every step envelope (including + // a duplicate from an accidental double wrap) is sent before this request's + // own transaction completes the test's expectations. + for (let i = 0; i < 20; i++) { + try { + const s = await instance.status(); + if (s.status === 'complete' || s.status === 'errored') { + return Response.json({ id: instance.id, ...s }); + } + } catch { + // status() may not be available in local dev + } + await new Promise(resolve => setTimeout(resolve, 250)); + } + return Response.json({ id: instance.id, status: 'timeout' }); + } + + return new Response('Not found', { status: 404 }); + }, +} satisfies ExportedHandler; diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/instrument.server.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/instrument.server.ts new file mode 100644 index 000000000000..4355b90010d6 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/instrument.server.ts @@ -0,0 +1,7 @@ +import { defineCloudflareOptions } from '@sentry/cloudflare'; + +export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({ + dsn: env.SENTRY_DSN, + traceLifecycle: 'static', + tracesSampleRate: 1.0, +})); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/test.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/test.ts new file mode 100644 index 000000000000..6f072a3aac4f --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/test.ts @@ -0,0 +1,48 @@ +import type { TransactionEvent } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { createRunner } from '../../../runner'; + +// `MyWorkflow` is hand-wrapped in `./workflow` and only re-exported by the +// entry, so the transform's emitted `_INTERNAL_wrapUnlessInstrumented` guard +// must hand the manual wrap back instead of nesting a second wrapper. Nested +// workflow wrappers each run the step through their own client, producing TWO +// identical `step-one` transactions, each individually well-formed, so the +// real check is the count assertion at the end. +// +// Ordering is anchored by a sentinel rather than by waiting: `/trigger` +// responds only after the workflow finished (every step envelope, including a +// duplicate, is flushed before then), and `/sentinel` is requested after that, +// so its transaction arrives a full request/response cycle behind any +// duplicate. Once the sentinel envelope has been matched, everything sent +// before it is known to have been delivered. +it('does not double-instrument an imported, already-wrapped Workflow', async ({ signal }) => { + const runner = createRunner(__dirname) + .unordered() + .expect(envelope => { + const transactionEvent = envelope[1]?.[0]?.[1] as TransactionEvent; + expect(transactionEvent.transaction).toBe('step-one'); + expect(transactionEvent.contexts?.trace?.op).toBe('function.step.do'); + expect(transactionEvent.contexts?.trace?.origin).toBe('auto.faas.cloudflare.workflow'); + }) + .expect(envelope => { + const transactionEvent = envelope[1]?.[0]?.[1] as TransactionEvent; + // The auto-wrapped default export's own transaction. + expect(transactionEvent.transaction).toBe('GET /trigger'); + expect(transactionEvent.contexts?.trace?.op).toBe('http.server'); + }) + // The sentinel is part of the expected set, so the runner keeps everything + // alive (and keeps receiving envelopes) until it has arrived. + .expect(envelope => { + expect((envelope[1]?.[0]?.[1] as TransactionEvent).transaction).toBe('GET /sentinel'); + }) + .start(signal); + + await runner.makeRequest('get', '/trigger'); + await runner.makeRequest('get', '/sentinel'); + await runner.completed(); + + const stepTransactions = runner + .getReceivedEnvelopes() + .filter(envelope => (envelope[1]?.[0]?.[1] as TransactionEvent | undefined)?.transaction === 'step-one'); + expect(stepTransactions).toHaveLength(1); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/vite.config.mts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/vite.config.mts new file mode 100644 index 000000000000..49cd4b297b8e --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/vite.config.mts @@ -0,0 +1,16 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + // The Sentry plugin runs first so its build-time transform wraps the worker + // entry and the `MyWorkflow` class before the Cloudflare plugin bundles it. + plugins: [ + cloudflare(), + sentryCloudflareVitePlugin({ + _experimental: { + autoInstrumentation: true, + }, + }), + ], +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/workflow.ts b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/workflow.ts new file mode 100644 index 000000000000..cc74ee7dfa68 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/workflow.ts @@ -0,0 +1,20 @@ +import * as Sentry from '@sentry/cloudflare'; +import { WorkflowEntrypoint } from 'cloudflare:workers'; +import type { WorkflowEvent, WorkflowStep } from 'cloudflare:workers'; + +interface Env { + SENTRY_DSN: string; +} + +class MyWorkflowImpl extends WorkflowEntrypoint { + async run(_event: WorkflowEvent, step: WorkflowStep): Promise { + await step.do('step-one', async () => 'done'); + } +} + +// Manually instrumented here, in a module separate from the worker entry, which +// only imports and re-exports the wrapped class. +export const MyWorkflow = Sentry.instrumentWorkflowWithSentry( + (env: Env) => ({ dsn: env.SENTRY_DSN, traceLifecycle: 'static', tracesSampleRate: 1.0 }), + MyWorkflowImpl, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/wrangler.jsonc new file mode 100644 index 000000000000..faad7d585af6 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/vite-autoinstrument/workflow-reexport-instrumented/wrangler.jsonc @@ -0,0 +1,16 @@ +{ + "$schema": "../../../node_modules/wrangler/config-schema.json", + "name": "cloudflare-vite-autoinstrument-workflow-reexport-instrumented", + // `main` points at the source entry; the Sentry Vite plugin builds from it (so + // the auto-instrument transform runs) and the runner serves the built output. + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], + "workflows": [ + { + "name": "my-workflow-reexport", + "binding": "MY_WORKFLOW", + "class_name": "MyWorkflow", + }, + ], +} diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/imported-agent.ts b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/imported-agent.ts new file mode 100644 index 000000000000..55c32332481a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/imported-agent.ts @@ -0,0 +1,17 @@ +import { Agent, callable } from 'agents'; + +/** + * An Agent declared outside the worker entry, which imports it and exports it again by specifier + * (`import { ImportedAgent } from './imported-agent'; export { ImportedAgent }`). The entry has no + * local class to rename, so the plugin has to re-point the export at a wrapper binding instead. + */ +export class ImportedAgent extends Agent { + @callable() + async greet(name: string): Promise { + return `Hello, ${name}! (from ImportedAgent)`; + } + + async onRequest(): Promise { + return Response.json({ agent: 'imported' }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/index.ts b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/index.ts index 5026f1fbd13b..c79bfad0c0d9 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/index.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/index.ts @@ -2,12 +2,20 @@ import { AIChatAgent } from '@cloudflare/ai-chat'; import { Agent, callable, routeAgentRequest } from 'agents'; import { DurableObject } from 'cloudflare:workers'; import { MyBase } from './base'; +import { ImportedAgent } from './imported-agent'; +export { ImportedAgent }; +export { ReExportedAgent } from './reexported-agent'; + +// The two exports above live in their own modules — see `imported-agent.ts` and +// `reexported-agent.ts`. They cover the shapes an entry that only aggregates +// classes uses, where there is no local declaration for the plugin to rewrite. +// // NOTE: this file deliberately contains NO `Sentry.*` calls and no import of // `@sentry/cloudflare`. Everything below is wrapped at build time by // `sentryCloudflareVitePlugin({ _experimental: { autoInstrumentation: true } })`, // which reads wrangler.jsonc, wraps the default export with `withSentry`, and -// picks a wrapper per class: `instrumentAgentWithSentry` for the three Agents, +// picks a wrapper per class: `instrumentAgentWithSentry` for the five Agents, // `instrumentDurableObjectWithSentry` for the plain Durable Object. // // Options come from `instrument.server.ts` next to this entry. diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/reexported-agent.ts b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/reexported-agent.ts new file mode 100644 index 000000000000..b0ac13239a4a --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/src/reexported-agent.ts @@ -0,0 +1,17 @@ +import { Agent, callable } from 'agents'; + +/** + * An Agent the worker entry only ever re-exports (`export { ReExportedAgent } from + * './reexported-agent'`) — it never binds the class locally at all, so the plugin has to import it + * under a private name before it can wrap it. + */ +export class ReExportedAgent extends Agent { + @callable() + async greet(name: string): Promise { + return `Hello, ${name}! (from ReExportedAgent)`; + } + + async onRequest(): Promise { + return Response.json({ agent: 'reexported' }); + } +} diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/tests/autoinstrument.test.ts b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/tests/autoinstrument.test.ts index 4a8d4d8f3bce..9652af21b1ac 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/tests/autoinstrument.test.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/tests/autoinstrument.test.ts @@ -45,6 +45,16 @@ for (const { title, binding, agentClass } of [ binding: 'derived-agent', agentClass: 'DerivedAgent', }, + { + title: 'an Agent imported from another module and exported by specifier', + binding: 'imported-agent', + agentClass: 'ImportedAgent', + }, + { + title: 'an Agent re-exported straight from another module', + binding: 're-exported-agent', + agentClass: 'ReExportedAgent', + }, ]) { test(`applies agent instrumentation to ${title}`, async ({ baseURL }) => { const instance = `${binding}-instance`; diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/wrangler.jsonc b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/wrangler.jsonc index d3765273c352..86488258140b 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/wrangler.jsonc +++ b/dev-packages/e2e-tests/test-applications/cloudflare-autoinstrument/wrangler.jsonc @@ -10,15 +10,19 @@ // "this one is an Agent". Only the base-class chain distinguishes them, which // is exactly what the plugin's detection has to work out at build time: // - // MyAgent -> extends Agent (entry-local) => agent - // MyChatAgent -> extends AIChatAgent (entry-local) => agent - // DerivedAgent -> extends ./base#MyBase -> Agent => agent - // PlainDO -> extends DurableObject => durableObject + // MyAgent -> extends Agent (entry-local) => agent + // MyChatAgent -> extends AIChatAgent (entry-local) => agent + // DerivedAgent -> extends ./base#MyBase -> Agent => agent + // ImportedAgent -> imported, then `export { ImportedAgent }` => agent + // ReExportedAgent -> `export { ... } from './reexported-agent'` => agent + // PlainDO -> extends DurableObject => durableObject "durable_objects": { "bindings": [ { "name": "MyAgent", "class_name": "MyAgent" }, { "name": "MyChatAgent", "class_name": "MyChatAgent" }, { "name": "DerivedAgent", "class_name": "DerivedAgent" }, + { "name": "ImportedAgent", "class_name": "ImportedAgent" }, + { "name": "ReExportedAgent", "class_name": "ReExportedAgent" }, { "name": "PlainDO", "class_name": "PlainDO" }, ], }, @@ -28,5 +32,9 @@ "tag": "v1", "new_sqlite_classes": ["MyAgent", "MyChatAgent", "DerivedAgent", "PlainDO"], }, + { + "tag": "v2", + "new_sqlite_classes": ["ImportedAgent", "ReExportedAgent"], + }, ], } diff --git a/packages/cloudflare/src/durableobject.ts b/packages/cloudflare/src/durableobject.ts index 6077b1881235..39e4f4106ca2 100644 --- a/packages/cloudflare/src/durableobject.ts +++ b/packages/cloudflare/src/durableobject.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ /* eslint-disable @typescript-eslint/unbound-method */ import { isObjectLike } from '@sentry/core'; import type { DurableObject } from 'cloudflare:workers'; @@ -377,7 +378,7 @@ export function instrumentDurableObjectWithSentry< T extends DurableObject, C extends new (state: DurableObjectState, env: E) => T, >(optionsCallback: (env: E) => CloudflareOptions, DurableObjectClass: C): C { - return new Proxy(DurableObjectClass, { + const InstrumentedClass = new Proxy(DurableObjectClass, { construct(target, [ctx, env], newTarget) { const { obj, options, context, frameworkManagedMethods } = constructInstrumentedDurableObject( target, @@ -390,6 +391,9 @@ export function instrumentDurableObjectWithSentry< return finalizeWithRpcInstrumentation(obj, options, context, frameworkManagedMethods); }, }); + // Recognizable for `_INTERNAL_wrapUnlessInstrumented`, so auto-instrumentation never nests wrappers. + markAsInstrumented(InstrumentedClass); + return InstrumentedClass; } /** @@ -437,7 +441,7 @@ export function instrumentAgentWithSentry< T extends DurableObject, C extends new (state: DurableObjectState, env: E) => T, >(optionsCallback: (env: E) => CloudflareOptions, AgentClass: C): C { - return new Proxy(AgentClass, { + const InstrumentedClass = new Proxy(AgentClass, { construct(target, [ctx, env], newTarget) { const { obj, options, context, frameworkManagedMethods } = constructInstrumentedDurableObject( target, @@ -456,4 +460,6 @@ export function instrumentAgentWithSentry< return finalizeWithRpcInstrumentation(obj, options, context, frameworkManagedMethods); }, }); + markAsInstrumented(InstrumentedClass); + return InstrumentedClass; } diff --git a/packages/cloudflare/src/index.ts b/packages/cloudflare/src/index.ts index 1e1896d6bc95..9cf7189fec21 100644 --- a/packages/cloudflare/src/index.ts +++ b/packages/cloudflare/src/index.ts @@ -121,6 +121,7 @@ export { export { withSentry } from './withSentry'; export { defineCloudflareOptions } from './defineCloudflareOptions'; export { instrumentAgentWithSentry, instrumentDurableObjectWithSentry } from './durableobject'; +export { _INTERNAL_wrapUnlessInstrumented } from './instrument'; export { sentryPagesPlugin } from './pages-plugin'; export { wrapRequestHandler } from './request'; diff --git a/packages/cloudflare/src/instrument.ts b/packages/cloudflare/src/instrument.ts index 6a19a61b6a0a..6e8e236ae276 100644 --- a/packages/cloudflare/src/instrument.ts +++ b/packages/cloudflare/src/instrument.ts @@ -84,3 +84,20 @@ export function ensureInstrumented(original: T, instrumentFn: (original: T) = return instrumented; } + +/** + * Applies `wrap` to `Class` unless `Class` is already an instrumented wrapper. + * + * Emitted by the Vite auto-instrumentation transform for classes the worker entry only re-exports: + * such a class may already be hand-wrapped with `instrument*WithSentry` in the module it comes + * from, which the build cannot see. Wrapping again would nest two proxies and instrument the same + * work twice, duplicated child spans, so the hand-wrapped class is returned as-is. Not part of the + * public API, hand-written wrapper calls are deliberately left untouched by this guard. + */ +export function _INTERNAL_wrapUnlessInstrumented( + wrap: (optionsCallback: (env: never) => unknown, Class: C) => C, + optionsCallback: (env: never) => unknown, + Class: C, +): C { + return getInstrumented(Class) === Class ? Class : wrap(optionsCallback, Class); +} diff --git a/packages/cloudflare/src/instrumentations/instrumentWorkerEntrypoint.ts b/packages/cloudflare/src/instrumentations/instrumentWorkerEntrypoint.ts index bfb215d2e034..087d1405ba9b 100644 --- a/packages/cloudflare/src/instrumentations/instrumentWorkerEntrypoint.ts +++ b/packages/cloudflare/src/instrumentations/instrumentWorkerEntrypoint.ts @@ -1,6 +1,7 @@ import type { RpcStub, WorkerEntrypoint } from 'cloudflare:workers'; import { setAsyncLocalStorageAsyncContextStrategy } from '../async'; import type { CloudflareOptions } from '../client'; +import { markAsInstrumented } from '../instrument'; import { getFinalOptions } from '../options'; import { instrumentContext } from '../utils/instrumentContext'; import { extractRpcMeta } from '../utils/rpcMeta'; @@ -159,7 +160,7 @@ export function instrumentWorkerEntrypoint< // each time, breaking scope isolation for concurrent requests setAsyncLocalStorageAsyncContextStrategy(); - return new Proxy(WorkerEntrypointClass, { + const InstrumentedClass = new Proxy(WorkerEntrypointClass, { construct(target, [ctx, env]) { const context = instrumentContext(ctx); const options = getFinalOptions(optionsCallback(env), env); @@ -235,4 +236,7 @@ export function instrumentWorkerEntrypoint< return proxy; }, }); + // Recognizable for `_INTERNAL_wrapUnlessInstrumented`, so auto-instrumentation never nests wrappers. + markAsInstrumented(InstrumentedClass); + return InstrumentedClass; } diff --git a/packages/cloudflare/src/vite/agentClass.ts b/packages/cloudflare/src/vite/agentClass.ts index e19118d29280..d335a7ad44a2 100644 --- a/packages/cloudflare/src/vite/agentClass.ts +++ b/packages/cloudflare/src/vite/agentClass.ts @@ -80,21 +80,25 @@ export async function detectAgentClasses( } /** - * The local class names in the entry that a configured class name could refer to — either declared - * under that name directly, or aliased to it by an `export { Local as Configured }` specifier. + * The entry-module binding names a configured class name could refer to — a class declared under + * that name, an import of it from another module, a `export { X } from './x'` re-export, or the + * local binding an `export { Local as Configured }` specifier aliases. * - * Keeps detection (which reads and scans other modules) off classes that no binding points at. + * Keeps detection (which reads and scans other modules) off names no binding points at. */ export function collectAgentCandidates(ast: ProgramBody, configuredNames: Iterable): Set { const shape = shapeFromAst(ast); const candidates = new Set(); + const isResolvable = (name: string): boolean => + shape.classes.has(name) || shape.imports.has(name) || shape.reexports.has(name); + for (const configured of configuredNames) { - if (shape.classes.has(configured)) { + if (isResolvable(configured)) { candidates.add(configured); } const local = shape.localExports.get(configured); - if (local && shape.classes.has(local)) { + if (local && isResolvable(local)) { candidates.add(local); } } diff --git a/packages/cloudflare/src/vite/autoInstrument.ts b/packages/cloudflare/src/vite/autoInstrument.ts index 707eed54375e..6c7e8f9734fc 100644 --- a/packages/cloudflare/src/vite/autoInstrument.ts +++ b/packages/cloudflare/src/vite/autoInstrument.ts @@ -135,9 +135,10 @@ export function sentryCloudflareAutoInstrumentPlugin(options: { wranglerConfigPa const missing = [...classWrappers.keys()].filter(name => !wrappedClasses.has(name)); if (missing.length > 0) { this.warn?.( - `[sentry] Could not auto-instrument class(es) ${missing.join(', ')}: no matching exported class ` + - 'declaration found in the worker entry (re-exports from other modules cannot be wrapped ' + - 'automatically). Wrap them manually with the matching `instrument*WithSentry` helper.', + `[sentry] Could not auto-instrument ${missing.join(', ')}.` + + 'The worker entry has no export matching them. ' + + 'Star re-exports (`export * from "./do"`) are not matched. ' + + 'Export them by name or wrap them manually with corresponding `instrument*WithSentry` helpers.', ); } diff --git a/packages/cloudflare/src/vite/transform.ts b/packages/cloudflare/src/vite/transform.ts index 9d44b7413799..915955ad51f8 100644 --- a/packages/cloudflare/src/vite/transform.ts +++ b/packages/cloudflare/src/vite/transform.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ import MagicString from 'magic-string'; import { DEFAULT_EXPORT, type ExportName, type SameWorkerBinding } from './bindings'; import { detectWorkerEntrypointClasses } from './workerEntrypoint'; @@ -40,15 +41,14 @@ interface ExportDefaultNode extends BaseNode { declaration: BaseNode; } -interface ExportSpecifierNode { - type: string; +interface ExportSpecifierNode extends BaseNode { local?: { type: string; name?: string }; exported?: { type: string; name?: string }; } interface ExportNamedNode extends BaseNode { declaration?: BaseNode | null; - source?: unknown; + source?: BaseNode | null; specifiers?: ExportSpecifierNode[]; } @@ -84,6 +84,9 @@ function isCallToMethod(node: BaseNode, methodName: string): boolean { */ export type ClassWrapperKind = 'durableObject' | 'agent' | 'workflow' | 'workerEntrypoint'; +/** Specifier name a `export { default as X } from '...'` re-export uses for the source's default. */ +const DEFAULT_IMPORT = 'default'; + /** * The `@sentry/cloudflare` helper each wrapper kind emits. All share the same * `(optionsCallback, Class)` signature. `WorkerEntrypoint` classes use @@ -132,10 +135,11 @@ export interface TransformResult { * {@link TransformContext.classWrappers}, e.g. Durable Object classes with * `instrumentDurableObjectWithSentry`). * - * Handles both `export class MyDO {}` and the specifier form - * (`class MyDO {}` … `export { MyDO }` / `export { Foo as MyDO }`). - * Re-exports from other modules (`export { MyDO } from './do'`) cannot be - * wrapped here and are left alone — the plugin warns about them via + * Handles `export class MyDO {}`, the specifier form (`class MyDO {}` … + * `export { MyDO }` / `export { Foo as MyDO }`), and classes that live in + * another module (`import { MyDO } from './do'; export { MyDO }` or + * `export { MyDO } from './do'`). Only star re-exports (`export * from './do'`) + * are left alone — the plugin warns about those via * {@link TransformResult.wrappedClasses}. * * Exported (rather than inlined into the plugin) so it can be unit-tested with a @@ -162,6 +166,7 @@ export function applyAutoInstrumentTransforms( // The identifier must be chosen before wrapping, which bindings survive is only known after. optionsFn: sameWorkerBindings.length > 0 ? MERGED_OPTIONS_IDENTIFIER : ctx.optionsFn, autoWrapped: new Set(), + manuallyWrappedLocals: collectManuallyWrappedLocals(ast), }; const { wrappedClasses } = state; @@ -232,6 +237,12 @@ interface TransformState { * wrapped with. `wrappedClasses` counts both. */ autoWrapped: Set; + /** + * Top-level bindings already assigned an `instrument*WithSentry(...)` result + * (`const MyDO = instrumentDurableObjectWithSentry(...)`). Exporting one by + * specifier must report it as wrapped rather than wrap it a second time. + */ + manuallyWrappedLocals: Set; } /** @@ -274,6 +285,11 @@ function buildMergedOptionsDeclaration( * `WorkerEntrypoint` subclass (matched by its *local* name) gets wrapped with * `withSentry`. * + * `localName` is the binding **in this module** the export refers to, and must be + * left undefined for `export { X } from '...'`: there the specifier's name belongs + * to the source module, so matching it against anything detected here would wrap a + * class that merely shares a name. + * * The one case where config is refined rather than obeyed is an `agents` Agent: * it *is* a Durable Object, so wrangler can only ever describe it as one, and * only the detected base chain distinguishes the two. @@ -284,12 +300,41 @@ function resolveWrapperKind( state: TransformState, ): ClassWrapperKind | undefined { const configured = state.classWrappers.get(exportedName); - if (configured === 'durableObject' && localName && state.agentClasses.has(localName)) return 'agent'; + + if ( + configured === 'durableObject' && + ((localName && state.agentClasses.has(localName)) || state.agentClasses.has(exportedName)) + ) { + return 'agent'; + } + if (configured) return configured; if (localName && state.workerEntrypointClasses.has(localName)) return 'workerEntrypoint'; return undefined; } +/** + * Top-level `const X = instrument*WithSentry(...)` bindings — a hand-wrapped class that is exported + * separately (`export { X }`) rather than inline. + */ +function collectManuallyWrappedLocals(ast: ProgramBody): Set { + const wrapperMethods = Object.values(WRAPPER_METHODS); + const locals = new Set(); + + for (const node of ast.body) { + if (node.type !== 'VariableDeclaration') continue; + for (const declarator of (node as VariableDeclarationNode).declarations ?? []) { + const name = declarator.id?.type === 'Identifier' ? declarator.id.name : undefined; + const init = declarator.init; + if (name && init && wrapperMethods.some(method => isCallToMethod(init, method))) { + locals.add(name); + } + } + } + + return locals; +} + function collectTopLevelClasses(ast: ProgramBody): Map { const classes = new Map(); for (const node of ast.body) { @@ -340,12 +385,8 @@ function handleNamedExport(node: ExportNamedNode, ctx: TransformContext, state: return; } - // ---- Specifier export of a local class (`export { Foo as MyDO }`) ---- - // Re-exports from another module carry a `source` — nothing local to wrap. - if (node.source) return; - for (const specifier of node.specifiers ?? []) { - wrapSpecifierExport(specifier, ctx, state); - } + // ---- Specifier export (`export { Foo as MyDO }`, `export { MyDO } from './do'`) ---- + wrapSpecifierExports(node, ctx, state); } function collectManuallyWrappedClassExports( @@ -401,28 +442,133 @@ function wrapInlineClassExport( state.needsImport = true; } -function wrapSpecifierExport(specifier: ExportSpecifierNode, ctx: TransformContext, state: TransformState): void { - if (specifier.type !== 'ExportSpecifier' || specifier.exported?.type !== 'Identifier') return; - const exportedName = specifier.exported.name; - if (!exportedName) return; +/** + * Wrap the configured classes an `export { ... }` statement names. + * + * A class *declared* in this module keeps the statement intact: the declaration is renamed and the + * wrapper takes over its binding, so the untouched specifier now exports the wrapped class. + * + * A class that lives in **another** module — imported and re-exported, or re-exported directly — has + * no local binding to overwrite (import bindings are immutable). Those specifiers are re-pointed at + * a fresh wrapper binding instead, which means rebuilding the statement; specifiers this plugin has + * no business touching are carried over verbatim. + */ +function wrapSpecifierExports(node: ExportNamedNode, ctx: TransformContext, state: TransformState): void { + const specifiers = node.specifiers ?? []; + if (specifiers.length === 0) return; + + const sourceLiteral = node.source ? state.ms.original.slice(node.source.start, node.source.end) : undefined; + + const prelude: string[] = []; + const wrappedPairs: string[] = []; + const kept: string[] = []; + + for (const specifier of specifiers) { + const pair = wrapCrossModuleSpecifier(specifier, sourceLiteral, ctx, state, prelude); + if (pair) { + wrappedPairs.push(pair); + } else { + kept.push(state.ms.original.slice(specifier.start, specifier.end)); + } + } + if (wrappedPairs.length === 0) return; + + const statements = [...prelude, `export { ${wrappedPairs.join(', ')} };`]; + if (kept.length > 0) { + const clause = `export { ${kept.join(', ')} }`; + statements.push(sourceLiteral ? `${clause} from ${sourceLiteral};` : `${clause};`); + } + state.ms.overwrite(node.start, node.end, statements.join('\n')); +} + +/** + * Handle one export specifier, returning the `Wrapped as Exported` pair to emit when its class has + * to be wrapped through a fresh binding — the cross-module case. The import/wrapper statements that + * pair depends on are pushed onto `prelude`. + * + * Returns `undefined` when the specifier can stay exactly as written: it doesn't name a configured + * class, its class is declared locally (wrapped in place via {@link wrapLocalClassExport}, which + * takes over the binding the specifier already exports), or the binding is already hand-wrapped. + */ +function wrapCrossModuleSpecifier( + specifier: ExportSpecifierNode, + sourceLiteral: string | undefined, + ctx: TransformContext, + state: TransformState, + prelude: string[], +): string | undefined { + const exportedName = + specifier.type === 'ExportSpecifier' && specifier.exported?.type === 'Identifier' + ? specifier.exported.name + : undefined; const localName = specifier.local?.type === 'Identifier' ? specifier.local.name : undefined; - const kind = resolveWrapperKind(exportedName, localName, state); - if (!kind) return; + // With a `from` clause the specifier names an export of the *source* module, not a binding here. + const kind = exportedName + ? resolveWrapperKind(exportedName, sourceLiteral ? undefined : localName, state) + : undefined; + + if (!exportedName || !localName || !kind) return undefined; + + // Without a `from` clause the specifier points at a module-local binding, which may already be + // (or become) the wrapped class without touching the export statement itself. + if (!sourceLiteral) { + const localClass = state.topLevelClasses.get(localName); + + if (localClass?.id) { + wrapLocalClassExport(localName, localClass, kind, ctx, state); + state.wrappedClasses.add(exportedName); + state.needsImport = true; + return undefined; + } - const localClass = localName ? state.topLevelClasses.get(localName) : undefined; - if (!localName || !localClass?.id) return; + if (state.manuallyWrappedLocals.has(localName)) { + state.wrappedClasses.add(exportedName); + return undefined; + } + } + // The class comes from another module: bind it under a private name (for the `from` form, which + // has no local binding at all), wrap that, and export the wrapper under the configured name. + let target = localName; + + if (sourceLiteral) { + target = `__SENTRY_REEXPORT_${exportedName}__`; + prelude.push( + localName === DEFAULT_IMPORT + ? `import ${target} from ${sourceLiteral};` + : `import { ${localName} as ${target} } from ${sourceLiteral};`, + ); + } + + const wrappedName = `__SENTRY_WRAPPED_${exportedName}__`; + + // The class may already be hand-wrapped in its own module, which this transform cannot see. The + // emitted guard returns such a class as-is instead of nesting a second wrapper around it. + prelude.push( + `const ${wrappedName} = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.${WRAPPER_METHODS[kind]}, ${state.optionsFn}, ${target});`, + ); state.wrappedClasses.add(exportedName); state.autoWrapped.add(exportedName); state.needsImport = true; - if (state.renamedLocals.has(localName)) return; + + return `${wrappedName} as ${exportedName}`; +} + +/** Rename a locally declared class and rebind its original name to the wrapper. */ +function wrapLocalClassExport( + localName: string, + localClass: ClassDeclarationNode, + kind: ClassWrapperKind, + ctx: TransformContext, + state: TransformState, +): void { + const classId = localClass.id; + if (!classId || state.renamedLocals.has(localName)) return; state.renamedLocals.add(localName); const renamedClass = `__SENTRY_ORIGINAL_${localName}__`; - state.ms.overwrite(localClass.id.start, localClass.id.end, renamedClass); - // The existing `export { ... }` statement keeps exporting the (now - // wrapped) `localName` binding, so the wrapper is NOT exported here. + state.ms.overwrite(classId.start, classId.end, renamedClass); state.ms.appendLeft( localClass.end, `\nconst ${localName} = __SENTRY__.${WRAPPER_METHODS[kind]}(${state.optionsFn}, ${renamedClass});\n`, diff --git a/packages/cloudflare/src/workflows.ts b/packages/cloudflare/src/workflows.ts index db55e96b872b..8c4bdfe950dd 100644 --- a/packages/cloudflare/src/workflows.ts +++ b/packages/cloudflare/src/workflows.ts @@ -24,6 +24,7 @@ import type { import { setAsyncLocalStorageAsyncContextStrategy } from './async'; import type { CloudflareOptions } from './client'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; +import { markAsInstrumented } from './instrument'; import { instrumentEnv } from './instrumentations/worker/instrumentEnv'; import { addCloudResourceContext } from './scope-utils'; import { init } from './sdk'; @@ -199,7 +200,7 @@ export function instrumentWorkflowWithSentry< T extends WorkflowEntrypoint, // WorkflowEntrypoint type C extends new (ctx: ExecutionContext, env: E) => T, // Constructor type of the WorkflowEntrypoint class >(optionsCallback: (env: E) => CloudflareOptions, WorkFlowClass: C): C { - return new Proxy(WorkFlowClass, { + const InstrumentedClass = new Proxy(WorkFlowClass, { construct(target: C, args: [ctx: ExecutionContext, env: E], newTarget) { const [ctx, env] = args; const context = instrumentContext(ctx); @@ -242,4 +243,7 @@ export function instrumentWorkflowWithSentry< }); }, }); + // Recognizable for `_INTERNAL_wrapUnlessInstrumented`, so auto-instrumentation never nests wrappers. + markAsInstrumented(InstrumentedClass); + return InstrumentedClass; } diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index 111d12fa1054..2f0d2d5f747d 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -3,7 +3,7 @@ import type { Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { instrumentAgentWithSentry, instrumentDurableObjectWithSentry } from '../src'; -import { getInstrumented } from '../src/instrument'; +import { _INTERNAL_wrapUnlessInstrumented, getInstrumented } from '../src/instrument'; import { resetSdk } from './testUtils'; describe('instrumentDurableObjectWithSentry', () => { @@ -839,4 +839,14 @@ describe('instrumentDurableObjectWithSentry', () => { // Verify that exactly one flush call was made during this test expect(delta).toBe(1); }); + + // The wrappers mark what they return, so the guard the Vite auto-instrumentation emits can + // recognize a hand-wrapped class and hand it back instead of nesting a second wrapper. + it('marks returned classes so auto-instrumentation does not wrap them again', () => { + const optionsCallback = vi.fn().mockReturnValue({}); + for (const wrap of [instrumentDurableObjectWithSentry, instrumentAgentWithSentry]) { + const HandWrapped = wrap(optionsCallback, class {} as any); + expect(_INTERNAL_wrapUnlessInstrumented(wrap as any, optionsCallback, HandWrapped)).toBe(HandWrapped); + } + }); }); diff --git a/packages/cloudflare/test/instrument.test.ts b/packages/cloudflare/test/instrument.test.ts index 659fdeec6374..fdd9c6c3909f 100644 --- a/packages/cloudflare/test/instrument.test.ts +++ b/packages/cloudflare/test/instrument.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { getInstrumented, markAsInstrumented } from '../src/instrument'; +import { _INTERNAL_wrapUnlessInstrumented, getInstrumented, markAsInstrumented } from '../src/instrument'; // Clean up the global WeakMap between tests to avoid cross-test pollution const GLOBAL_KEY = '__SENTRY_INSTRUMENTED_MAP__' as const; @@ -193,4 +193,32 @@ describe('instrument', () => { expect(existing).toBe(proxy); }); }); + + // The guard the Vite auto-instrumentation emits around classes it cannot prove unwrapped at + // build time: a hand-wrapped class (marked by its wrapper) passes through untouched, anything + // else is wrapped. Only original-marked-as-itself counts, an original that merely *has* a + // wrapped counterpart must still be wrapped, wrapping the same base twice on purpose is valid. + describe('_INTERNAL_wrapUnlessInstrumented', () => { + const wrap = (_cb: unknown, cls: object): object => new Proxy(cls, {}); + + it('returns an already-instrumented class unchanged without calling the wrapper', () => { + const HandWrapped = class {}; + markAsInstrumented(HandWrapped); + + expect(_INTERNAL_wrapUnlessInstrumented(wrap, () => ({}), HandWrapped)).toBe(HandWrapped); + }); + + it('wraps a plain class', () => { + const Plain = class {}; + + expect(_INTERNAL_wrapUnlessInstrumented(wrap, () => ({}), Plain)).not.toBe(Plain); + }); + + it('wraps an original that only has an instrumented counterpart', () => { + const Original = class {}; + markAsInstrumented(Original, class {}); + + expect(_INTERNAL_wrapUnlessInstrumented(wrap, () => ({}), Original)).not.toBe(Original); + }); + }); }); diff --git a/packages/cloudflare/test/vite/agentClass.test.ts b/packages/cloudflare/test/vite/agentClass.test.ts index ccc25e83287b..90fbea795d90 100644 --- a/packages/cloudflare/test/vite/agentClass.test.ts +++ b/packages/cloudflare/test/vite/agentClass.test.ts @@ -280,8 +280,18 @@ describe('collectAgentCandidates', () => { expect(collectAgentCandidates(parseJS(code), ['ConfiguredAgent'])).toEqual(new Set(['LocalAgent'])); }); - it('returns nothing when no configured class is declared here', async () => { + it('returns a configured name re-exported from another module', async () => { const code = "export { MyAgent } from './agent';"; + expect(collectAgentCandidates(parseJS(code), ['MyAgent'])).toEqual(new Set(['MyAgent'])); + }); + + it('returns the local binding of a configured name imported from another module', async () => { + const code = ["import { Impl as MyAgent } from './agent';", 'export { MyAgent };'].join('\n'); + expect(collectAgentCandidates(parseJS(code), ['MyAgent'])).toEqual(new Set(['MyAgent'])); + }); + + it('returns nothing when no binding points at the configured name', async () => { + const code = "export * from './agent';"; expect(collectAgentCandidates(parseJS(code), ['MyAgent'])).toEqual(new Set()); }); }); diff --git a/packages/cloudflare/test/vite/autoInstrument.test.ts b/packages/cloudflare/test/vite/autoInstrument.test.ts index e98023d28405..448c676a231d 100644 --- a/packages/cloudflare/test/vite/autoInstrument.test.ts +++ b/packages/cloudflare/test/vite/autoInstrument.test.ts @@ -305,6 +305,63 @@ describe('sentryCloudflareAutoInstrumentPlugin', () => { expect(result.code).not.toContain('instrumentAgentWithSentry'); }); + it('wraps an Agent imported from another module and exported by specifier', async () => { + const tx = createAgentPlugin({ + 'agent.ts': ["import { Agent } from 'agents';", 'export class MyAgent extends Agent {}'].join('\n'), + }); + const code = ["import { MyAgent } from './agent';", 'export { MyAgent };'].join('\n'); + + const result = await tx(code); + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + 'const __SENTRY_OPTIONS__ = (env) => { const opts = (() => undefined)(env); return { ...opts, enableRpcTracePropagation: opts?.enableRpcTracePropagation ?? true, rpcTracePropagationBindings: ["MY_AGENT", ...(opts?.rpcTracePropagationBindings ?? [])] }; };', + "import { MyAgent } from './agent';", + 'const __SENTRY_WRAPPED_MyAgent__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentAgentWithSentry, __SENTRY_OPTIONS__, MyAgent);', + 'export { __SENTRY_WRAPPED_MyAgent__ as MyAgent };', + ].join('\n'), + ); + }); + + it('wraps an Agent re-exported straight from another module', async () => { + const tx = createAgentPlugin({ + 'agent.ts': ["import { Agent } from 'agents';", 'export class MyAgent extends Agent {}'].join('\n'), + }); + const code = "export { MyAgent } from './agent';"; + + const result = await tx(code); + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + 'const __SENTRY_OPTIONS__ = (env) => { const opts = (() => undefined)(env); return { ...opts, enableRpcTracePropagation: opts?.enableRpcTracePropagation ?? true, rpcTracePropagationBindings: ["MY_AGENT", ...(opts?.rpcTracePropagationBindings ?? [])] }; };', + "import { MyAgent as __SENTRY_REEXPORT_MyAgent__ } from './agent';", + 'const __SENTRY_WRAPPED_MyAgent__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentAgentWithSentry, __SENTRY_OPTIONS__, __SENTRY_REEXPORT_MyAgent__);', + 'export { __SENTRY_WRAPPED_MyAgent__ as MyAgent };', + ].join('\n'), + ); + }); + + it('keeps the Durable Object helper for a re-exported plain Durable Object', async () => { + const tx = createAgentPlugin({ + 'do.ts': [ + "import { DurableObject } from 'cloudflare:workers';", + 'export class MyAgent extends DurableObject {}', + ].join('\n'), + }); + const code = "export { MyAgent } from './do';"; + + const result = await tx(code); + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + 'const __SENTRY_OPTIONS__ = (env) => { const opts = (() => undefined)(env); return { ...opts, enableRpcTracePropagation: opts?.enableRpcTracePropagation ?? true, rpcTracePropagationBindings: ["MY_AGENT", ...(opts?.rpcTracePropagationBindings ?? [])] }; };', + "import { MyAgent as __SENTRY_REEXPORT_MyAgent__ } from './do';", + 'const __SENTRY_WRAPPED_MyAgent__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, __SENTRY_OPTIONS__, __SENTRY_REEXPORT_MyAgent__);', + 'export { __SENTRY_WRAPPED_MyAgent__ as MyAgent };', + ].join('\n'), + ); + }); + it('does not warn about an Agent that was wrapped manually', async () => { const dir = writeTempDir({ 'wrangler.toml': AGENT_WRANGLER }); const plugin = sentryCloudflareAutoInstrumentPlugin(); @@ -342,7 +399,8 @@ describe('sentryCloudflareAutoInstrumentPlugin', () => { plugin.configResolved({ root: dir }); const warnings: string[] = []; - const code = "export { MyDO } from './do';"; + // A star re-export names nothing, so there is no specifier to re-point at a wrapper. + const code = "export * from './do';"; await plugin.transform.call( { parse: (c: string) => parseJS(c), warn: (msg: string) => warnings.push(msg) }, code, diff --git a/packages/cloudflare/test/vite/transform.test.ts b/packages/cloudflare/test/vite/transform.test.ts index 8dcc594c25f8..1ab71b9b0ba7 100644 --- a/packages/cloudflare/test/vite/transform.test.ts +++ b/packages/cloudflare/test/vite/transform.test.ts @@ -23,7 +23,11 @@ function entrypointWrappers(...names: string[]): Map { } function transform(code: string, ctx: TransformContext) { - return applyAutoInstrumentTransforms(code, parseJS(code), ctx); + const result = applyAutoInstrumentTransforms(code, parseJS(code), ctx); + // Rewriting whole statements (rather than only splicing in wrappers) makes it possible to emit + // syntactically broken output, which would surface as an opaque bundler error. + if (result) parseJS(result.code); + return result; } // --------------------------------------------------------------------------- @@ -203,9 +207,122 @@ describe('Durable Object class wrapping', () => { expect(result.wrappedClasses).toEqual(new Set(['MyDurableObject'])); }); - it('leaves re-exports from other modules alone and reports them unwrapped', () => { + it('wraps a DO class imported from another module and exported by specifier', () => { + const code = ["import { MyDurableObject } from './do';", 'export { MyDurableObject };'].join('\n'); + + const result = transform(code, ctx)!; + // The import binding cannot be reassigned, so the export is re-pointed at a wrapper binding. + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import { MyDurableObject } from './do';", + 'const __SENTRY_WRAPPED_MyDurableObject__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), MyDurableObject);', + 'export { __SENTRY_WRAPPED_MyDurableObject__ as MyDurableObject };', + ].join('\n'), + ); + expect(result.wrappedClasses).toEqual(new Set(['MyDurableObject'])); + }); + + it('wraps a DO class re-exported straight from another module', () => { const code = "export { MyDurableObject } from './do';"; - expect(transform(code, ctx)).toBeUndefined(); + + const result = transform(code, ctx)!; + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import { MyDurableObject as __SENTRY_REEXPORT_MyDurableObject__ } from './do';", + 'const __SENTRY_WRAPPED_MyDurableObject__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), __SENTRY_REEXPORT_MyDurableObject__);', + 'export { __SENTRY_WRAPPED_MyDurableObject__ as MyDurableObject };', + ].join('\n'), + ); + expect(result.wrappedClasses).toEqual(new Set(['MyDurableObject'])); + }); + + it('wraps an aliased re-export from another module', () => { + const code = "export { Internal as MyDurableObject } from './do';"; + + const result = transform(code, ctx)!; + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import { Internal as __SENTRY_REEXPORT_MyDurableObject__ } from './do';", + 'const __SENTRY_WRAPPED_MyDurableObject__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), __SENTRY_REEXPORT_MyDurableObject__);', + 'export { __SENTRY_WRAPPED_MyDurableObject__ as MyDurableObject };', + ].join('\n'), + ); + }); + + it('wraps a default re-export from another module', () => { + const code = "export { default as MyDurableObject } from './do';"; + + const result = transform(code, ctx)!; + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import __SENTRY_REEXPORT_MyDurableObject__ from './do';", + 'const __SENTRY_WRAPPED_MyDurableObject__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), __SENTRY_REEXPORT_MyDurableObject__);', + 'export { __SENTRY_WRAPPED_MyDurableObject__ as MyDurableObject };', + ].join('\n'), + ); + }); + + it('leaves sibling specifiers of a re-export statement untouched', () => { + const code = "export { Helper, MyDurableObject, other as Other } from './do';"; + + const result = transform(code, ctx)!; + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import { MyDurableObject as __SENTRY_REEXPORT_MyDurableObject__ } from './do';", + 'const __SENTRY_WRAPPED_MyDurableObject__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), __SENTRY_REEXPORT_MyDurableObject__);', + 'export { __SENTRY_WRAPPED_MyDurableObject__ as MyDurableObject };', + "export { Helper, other as Other } from './do';", + ].join('\n'), + ); + expect(result.wrappedClasses).toEqual(new Set(['MyDurableObject'])); + }); + + it('wraps a mix of local and imported classes in one export statement', () => { + const mixed: TransformContext = { classWrappers: doWrappers('LocalDO', 'ImportedDO'), optionsFn: '(env) => ({})' }; + const code = [ + "import { ImportedDO } from './do';", + 'class DurableObject {}', + 'class LocalDO extends DurableObject {}', + 'const unrelated = 1;', + 'export { LocalDO, ImportedDO, unrelated };', + ].join('\n'); + + const result = transform(code, mixed)!; + // The local class keeps its binding (renamed declaration + wrapper), so its specifier is + // carried over untouched alongside the unrelated one. + expect(result.code).toBe( + [ + "import * as __SENTRY__ from '@sentry/cloudflare';", + "import { ImportedDO } from './do';", + 'class DurableObject {}', + 'class __SENTRY_ORIGINAL_LocalDO__ extends DurableObject {}', + 'const LocalDO = __SENTRY__.instrumentDurableObjectWithSentry((env) => ({}), __SENTRY_ORIGINAL_LocalDO__);', + '', + 'const unrelated = 1;', + 'const __SENTRY_WRAPPED_ImportedDO__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, (env) => ({}), ImportedDO);', + 'export { __SENTRY_WRAPPED_ImportedDO__ as ImportedDO };', + 'export { LocalDO, unrelated };', + ].join('\n'), + ); + expect(result.wrappedClasses).toEqual(new Set(['LocalDO', 'ImportedDO'])); + }); + + it('counts a locally hand-wrapped class exported by specifier as wrapped', () => { + const code = [ + "import { instrumentDurableObjectWithSentry } from '@sentry/cloudflare';", + "import { Impl } from './do';", + 'const MyDurableObject = instrumentDurableObjectWithSentry((env) => ({}), Impl);', + 'export { MyDurableObject };', + ].join('\n'); + + const result = transform(code, ctx)!; + expect(result.wrappedClasses).toEqual(new Set(['MyDurableObject'])); + expect(result.code).toBe(code); }); it('reports wrapped DO classes for the inline export form', () => { @@ -284,6 +401,18 @@ describe('Workflow class wrapping', () => { expect(result.wrappedClasses).toEqual(new Set(['MyWorkflow'])); }); + it('wraps a workflow class re-exported from another module', () => { + const code = "export { MyWorkflow } from './workflow';"; + + const result = transform(code, ctx)!; + expect(result.code).toContain("import { MyWorkflow as __SENTRY_REEXPORT_MyWorkflow__ } from './workflow';"); + expect(result.code).toContain( + 'const __SENTRY_WRAPPED_MyWorkflow__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentWorkflowWithSentry, (env) => ({}), __SENTRY_REEXPORT_MyWorkflow__);', + ); + expect(result.code).toContain('export { __SENTRY_WRAPPED_MyWorkflow__ as MyWorkflow };'); + expect(result.code).not.toContain('instrumentDurableObjectWithSentry'); + }); + it('counts a manually wrapped workflow export as wrapped without touching it', () => { const code = [ "import { instrumentWorkflowWithSentry } from '@sentry/cloudflare';", @@ -416,11 +545,44 @@ describe('WorkerEntrypoint class wrapping (config fallback)', () => { expect(result.wrappedClasses).toEqual(new Set(['AdminEntry'])); }); + it('wraps a configured entrypoint re-exported from another module', () => { + const code = "export { AdminEntry } from './admin';"; + + const result = transform(code, ctx)!; + expect(result.code).toContain("import { AdminEntry as __SENTRY_REEXPORT_AdminEntry__ } from './admin';"); + expect(result.code).toContain( + 'const __SENTRY_WRAPPED_AdminEntry__ = __SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.withSentry, (env) => ({}), __SENTRY_REEXPORT_AdminEntry__);', + ); + expect(result.code).toContain('export { __SENTRY_WRAPPED_AdminEntry__ as AdminEntry };'); + }); + it('ignores an entrypoint that is neither structurally detected nor configured', () => { const other: TransformContext = { classWrappers: new Map(), optionsFn: '(env) => ({})' }; const code = ["import { BaseEntry } from './base';", 'export class AdminEntry extends BaseEntry {}'].join('\n'); expect(transform(code, other)).toBeUndefined(); }); + + // Structural detection reads the entry's own AST, so it can only ever name a class declared + // there — an unconfigured re-export has no base chain to inspect and must be left alone. + it('does not wrap a re-export that is only structurally detectable', () => { + const other: TransformContext = { classWrappers: new Map(), optionsFn: '(env) => ({})' }; + const code = "export { AdminEntry } from './admin';"; + expect(transform(code, other)).toBeUndefined(); + }); + + // In `export { X } from '...'` the specifier's "local" name belongs to the *source* module, so it + // must never be matched against classes detected in this one — they are unrelated bindings that + // merely share a name. + it('does not wrap a re-export whose source name collides with a local entrypoint class', () => { + const other: TransformContext = { classWrappers: new Map(), optionsFn: '(env) => ({})' }; + const code = [ + "import { WorkerEntrypoint } from 'cloudflare:workers';", + 'class AdminEntry extends WorkerEntrypoint {}', + "export { AdminEntry } from './admin';", + ].join('\n'); + + expect(transform(code, other)).toBeUndefined(); + }); }); // --------------------------------------------------------------------------- @@ -760,7 +922,7 @@ describe('same-worker RPC binding floor', () => { expect(result.code).not.toContain('rpcTracePropagationBindings'); }); - it('drops a binding whose class is re-exported from another module', () => { + it('keeps a binding whose class is re-exported from another module', () => { const code = ['export { MyDO } from "./myDo";', 'export default { fetch() {} };'].join('\n'); const result = transform(code, { @@ -769,8 +931,10 @@ describe('same-worker RPC binding floor', () => { sameWorkerBindings: [{ bindingName: 'MY_DO', className: 'MyDO' }], })!; - expect(result.code).toContain('const __SENTRY_OPTIONS__ = () => undefined;'); - expect(result.code).not.toContain('rpcTracePropagationBindings'); + expect(result.code).toContain('rpcTracePropagationBindings: ["MY_DO",'); + expect(result.code).toContain( + '__SENTRY__._INTERNAL_wrapUnlessInstrumented(__SENTRY__.instrumentDurableObjectWithSentry, __SENTRY_OPTIONS__, __SENTRY_REEXPORT_MyDO__)', + ); }); it('leaves the output untouched when there are no same-worker bindings', () => {