diff --git a/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/index.ts b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/index.ts new file mode 100644 index 000000000000..86eec9128c5b --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/index.ts @@ -0,0 +1,115 @@ +import * as Sentry from '@sentry/cloudflare'; +import { DurableObject } from 'cloudflare:workers'; + +interface Env { + SENTRY_DSN: string; + SCOPE_DO: DurableObjectNamespace; +} + +class ScopeDurableObjectBase extends DurableObject { + /** + * `setTag`/`setUser` write to the isolation scope, which a Durable Object keeps across + * invocations. Only the seeding invocation writes, so whatever a later invocation reports it + * must have inherited from a scope the two shared. + */ + async scopeCheck(seed: boolean): Promise { + if (seed) { + Sentry.setTag('seeded_tag', 'from-seeding-invocation'); + Sentry.setUser({ id: 'user-from-seeding-invocation' }); + } + + Sentry.captureException(new Error(seed ? 'Scope seed' : 'Scope probe')); + + return 'ok'; + } + + /** + * A direct method call on the same Durable Object is part of the calling invocation, so it + * must see — and be able to extend — the same isolation scope. Only the outer method captures: + * if the nested call ran in its own scope, the outer event would miss `inner_tag` and the user. + */ + async nestedScopeCheck(): Promise { + Sentry.setTag('outer_tag', 'from-outer-method'); + + await this.innerScopeHelper(); + + Sentry.captureException(new Error('Nested outer')); + + return 'ok'; + } + + async innerScopeHelper(): Promise { + Sentry.setTag('inner_tag', 'from-inner-method'); + Sentry.setUser({ id: 'user-from-inner-method' }); + } + + /** + * Same as `nestedScopeCheck`, but the nested call lands on `fetch` — an instrumented handler that + * opens an isolation scope of its own. Reaching it from inside another invocation must not fork + * again, or the nested handler would not see what the calling method set. + * + * The capture happens inside the nested call rather than after it: the nested handler tears its + * client down on the way out, so a capture in the calling method would have no transport left. + */ + async reentrantScopeCheck(): Promise { + Sentry.setTag('reentrant_outer_tag', 'from-rpc-method'); + Sentry.setUser({ id: 'user-from-rpc-method' }); + + await this.fetch(new Request('https://durable-object.invalid/inner')); + + return 'ok'; + } + + async fetch(_request: Request): Promise { + Sentry.setTag('fetch_tag', 'from-nested-fetch'); + Sentry.captureException(new Error('Reentrant inner')); + + // Deliberately bodyless. A `text/plain` body without a `content-length` is classified as + // streaming, and nothing here ever reads the nested response, so the span would stay open and + // hold up the flush. + return new Response(null, { status: 204 }); + } +} + +export const ScopeDurableObject = Sentry.instrumentDurableObjectWithSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + enableRpcTracePropagation: true, + }), + ScopeDurableObjectBase, +); + +export default Sentry.withSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1, + enableRpcTracePropagation: true, + }), + { + async fetch(request, env) { + const url = new URL(request.url); + + if (url.pathname === '/scope') { + // Always the same instance, so both invocations land on the same Durable Object. + const stub = env.SCOPE_DO.get(env.SCOPE_DO.idFromName('scope-do')) as DurableObjectStub; + + return new Response(await stub.scopeCheck(url.searchParams.get('seed') === '1')); + } + + if (url.pathname === '/nested') { + const stub = env.SCOPE_DO.get(env.SCOPE_DO.idFromName('scope-do')) as DurableObjectStub; + + return new Response(await stub.nestedScopeCheck()); + } + + if (url.pathname === '/reentrant') { + const stub = env.SCOPE_DO.get(env.SCOPE_DO.idFromName('scope-do')) as DurableObjectStub; + + return new Response(await stub.reentrantScopeCheck()); + } + + return new Response('Hello World!'); + }, + } satisfies ExportedHandler, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts new file mode 100644 index 000000000000..5da3d134d60a --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts @@ -0,0 +1,77 @@ +import type { Envelope, Event } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { createRunner } from '../../runner'; + +it('two consecutive invocations get different isolation scopes', async ({ signal }) => { + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + await runner.makeRequestAndWaitForEnvelope('get', '/scope?seed=1', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Scope seed'); + // Guards the probe assertions below against passing vacuously: the seeding invocation really + // did write to its isolation scope. + expect(event.tags).toEqual(expect.objectContaining({ seeded_tag: 'from-seeding-invocation' })); + expect(event.user).toEqual({ id: 'user-from-seeding-invocation' }); + }); + + await runner.makeRequestAndWaitForEnvelope('get', '/scope?seed=0', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Scope probe'); + expect(event.tags?.seeded_tag).toBeUndefined(); + expect(event.user).toBeUndefined(); + }); +}); + +it('a nested direct call within one invocation shares the same isolation scope', async ({ signal }) => { + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + await runner.makeRequestAndWaitForEnvelope('get', '/nested', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Nested outer'); + // The event must carry data written on both sides of the nested call: `outer_tag` from + // before it, `inner_tag` and the user from inside it — anything less means the nested + // call ran in its own scope. + expect(event.tags).toEqual( + expect.objectContaining({ + outer_tag: 'from-outer-method', + inner_tag: 'from-inner-method', + }), + ); + expect(event.user).toEqual({ id: 'user-from-inner-method' }); + }); + + // Whatever the nested invocation wrote must not survive into the next invocation. + await runner.makeRequestAndWaitForEnvelope('get', '/scope?seed=0', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Scope probe'); + expect(event.tags?.outer_tag).toBeUndefined(); + expect(event.tags?.inner_tag).toBeUndefined(); + expect(event.user).toBeUndefined(); + }); +}); + +it('a nested call into another instrumented handler shares the same isolation scope', async ({ signal }) => { + const runner = createRunner(__dirname).ignore('transaction', 'span').start(signal); + + await runner.makeRequestAndWaitForEnvelope('get', '/reentrant', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Reentrant inner'); + // `fetch` is itself instrumented and opens an isolation scope. Reached from inside the RPC + // invocation it must not fork again, or it would not see what the RPC method set. + expect(event.tags).toEqual( + expect.objectContaining({ + reentrant_outer_tag: 'from-rpc-method', + fetch_tag: 'from-nested-fetch', + }), + ); + expect(event.user).toEqual({ id: 'user-from-rpc-method' }); + }); + + await runner.makeRequestAndWaitForEnvelope('get', '/scope?seed=0', (envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Scope probe'); + expect(event.tags?.reentrant_outer_tag).toBeUndefined(); + expect(event.tags?.fetch_tag).toBeUndefined(); + expect(event.user).toBeUndefined(); + }); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/wrangler.jsonc new file mode 100644 index 000000000000..c1471a81a03a --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/wrangler.jsonc @@ -0,0 +1,15 @@ +{ + "name": "durable-object-scope-test", + "compatibility_date": "2025-06-17", + "main": "index.ts", + "compatibility_flags": ["nodejs_compat"], + "durable_objects": { + "bindings": [{ "name": "SCOPE_DO", "class_name": "ScopeDurableObject" }], + }, + "migrations": [ + { + "tag": "v1", + "new_sqlite_classes": ["ScopeDurableObject"], + }, + ], +} diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index e97d21c5c916..ff8ac0f11848 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -10,7 +10,6 @@ import { setHttpStatus, startSpanManual, winterCGHeadersToDict, - withIsolationScope, } from '@sentry/core'; import { captureIncomingRequestBody } from './integrations/httpServer'; import { initBaseSdk } from './baseSdk'; @@ -18,6 +17,7 @@ import type { CloudflareClient, CloudflareOptions } from './client'; import type { ExecutionContextCompat } from './executionContext'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils'; +import { withInvocationIsolationScope } from './utils/invocationScope'; import { classifyResponseStreaming } from './utils/streaming'; function getRequestErrorMechanismType(context: ExecutionContextCompat | undefined): string { @@ -72,7 +72,7 @@ export function wrapRequestHandlerWithInit( handler: (...args: unknown[]) => Response | Promise, initSdk: InitSdk, ): Promise { - return withIsolationScope(async isolationScope => { + return withInvocationIsolationScope(async isolationScope => { const { options, request, captureErrors = true } = wrapperOptions; const context = wrapperOptions.context; diff --git a/packages/cloudflare/src/utils/invocationScope.ts b/packages/cloudflare/src/utils/invocationScope.ts new file mode 100644 index 000000000000..3591bac64eb2 --- /dev/null +++ b/packages/cloudflare/src/utils/invocationScope.ts @@ -0,0 +1,30 @@ +import { getDefaultIsolationScope, getIsolationScope, type Scope, withIsolationScope } from '@sentry/core'; + +/** + * Runs `callback` on the isolation scope for the current invocation. + * + * An instrumented handler is either the entry point of an invocation or reentrant — reached from + * another instrumented handler already serving the same invocation (a Durable Object method calling + * its own `fetch`, an RPC method reaching a sibling method). Only the entry point may fork: + * + * - Forking at the entry point is mandatory. `setUser`/`setTag` write to the isolation scope, and a + * Durable Object's isolation scope outlives the invocation that touched it, so without a fork one + * invocation's user and tags reappear on the next invocation's events in the same isolate. + * Forking clones, so request data set by an enclosing wrapper is still inherited. + * - Forking again when reentrant would be wrong. Everything below the entry point is one logical + * unit of work: a nested call must see what the caller set and be able to add to it, the way it + * would if the SDK were not wrapping it at all. + * + * The AsyncLocalStorage strategy hands the default isolation scope back whenever no invocation is in + * flight, and a forked one while inside `withIsolationScope`. Reference-comparing against the default + * is therefore enough to tell the two cases apart. The stack fallback does not fork, so it reports the + * default scope even inside an invocation; there the fork degrades to a no-op, which the stack strategy + * tolerates. This matches the approach used by `patchEventHandler` in Nuxt. + */ +export function withInvocationIsolationScope(callback: (scope: Scope) => T): T { + const isolationScope = getIsolationScope(); + + const newIsolationScope = isolationScope === getDefaultIsolationScope() ? isolationScope.clone() : isolationScope; + + return withIsolationScope(newIsolationScope, () => callback(newIsolationScope)); +} diff --git a/packages/cloudflare/src/wrapMethodWithSentry.ts b/packages/cloudflare/src/wrapMethodWithSentry.ts index 4723887dbace..3484d8c07394 100644 --- a/packages/cloudflare/src/wrapMethodWithSentry.ts +++ b/packages/cloudflare/src/wrapMethodWithSentry.ts @@ -4,21 +4,19 @@ import { isObjectLike, captureException, continueTrace, - getClient, isThenable, type Scope, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startNewTrace as startNewTraceCore, startSpan, - withIsolationScope, - withScope, } from '@sentry/core'; import type { CloudflareOptions } from './client'; import type { ExecutionContextCompat } from './executionContext'; import { flushAndDispose, getOriginalWaitUntil } from './flush'; import { ensureInstrumented } from './instrument'; import { init } from './sdk'; +import { withInvocationIsolationScope } from './utils/invocationScope'; import { extractRpcMeta } from './utils/rpcMeta'; import { buildSpanLinks, getStoredSpanContext, storeSpanContext } from './utils/traceLinks'; @@ -112,11 +110,6 @@ export function wrapMethodWithSentry( rpcMeta = extracted.rpcMeta; } - // For startNewTrace, always use withIsolationScope to ensure a fresh scope - // Otherwise, use existing client's scope or isolation scope - const currentClient = getClient(); - const sentryWithScope = startNewTrace ? withIsolationScope : currentClient ? withScope : withIsolationScope; - const wrappedFunction = (scope: Scope): unknown | Promise => { // In certain situations, the passed context can become undefined. // For example, for Astro while prerendering pages at build time. @@ -241,7 +234,7 @@ export function wrapMethodWithSentry( return executeSpan(); }; - return sentryWithScope(wrappedFunction); + return withInvocationIsolationScope(wrappedFunction); }, }), noMark, diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index ec0c9e8ec708..8d4fb222cb68 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -1,12 +1,15 @@ import type { ExecutionContext } from '@cloudflare/workers-types'; +import type { Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { instrumentDurableObjectWithSentry } from '../src'; import { getInstrumented } from '../src/instrument'; +import { resetSdk } from './testUtils'; describe('instrumentDurableObjectWithSentry', () => { afterEach(() => { vi.restoreAllMocks(); + resetSdk(); }); it('Generic functionality', () => { @@ -197,6 +200,66 @@ describe('instrumentDurableObjectWithSentry', () => { expect(obj.method).toBe(obj.method); }); + // Hibernation-woken WebSocket messages and alarms arrive as their own invocations with no + // enclosing instrumented handler, so each must open a fresh isolation scope. The Durable Object + // instance outlives them, so a leak here would follow the isolate for its remaining lifetime. + it('Runtime-invoked built-in handlers each get their own isolation scope', async () => { + const events: Event[] = []; + const waits: Promise[] = []; + const mockContext = { + waitUntil: vi.fn((promise: Promise) => { + waits.push(promise); + }), + } as any; + + const testClass = class { + webSocketMessage(_ws: unknown, message: string) { + if (message === 'seed') { + SentryCore.setTag('seeded_tag', 'from-seeding-message'); + SentryCore.setUser({ id: 'user-from-seeding-message' }); + } + + SentryCore.captureMessage(message); + } + + alarm() { + SentryCore.captureMessage('alarm'); + } + }; + const obj = Reflect.construct( + instrumentDurableObjectWithSentry( + () => ({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + beforeSend(event: Event) { + events.push(event); + return null; + }, + }), + testClass as any, + ), + [mockContext, {} as any], + ); + + await obj.webSocketMessage({}, 'seed'); + await Promise.all(waits.splice(0)); + await obj.webSocketMessage({}, 'probe'); + await Promise.all(waits.splice(0)); + await obj.alarm(); + await Promise.all(waits); + + // Guards the assertions below against passing vacuously. + expect(events[0]?.tags).toEqual(expect.objectContaining({ seeded_tag: 'from-seeding-message' })); + expect(events[0]?.user).toEqual({ id: 'user-from-seeding-message' }); + + expect(events[1]?.message).toBe('probe'); + expect(events[1]?.tags?.seeded_tag).toBeUndefined(); + expect(events[1]?.user).toBeUndefined(); + + expect(events[2]?.message).toBe('alarm'); + expect(events[2]?.tags?.seeded_tag).toBeUndefined(); + expect(events[2]?.user).toBeUndefined(); + }); + it('Built-in durable object methods are always instrumented', () => { const testClass = class { fetch() {} diff --git a/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts b/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts index 7f23f4cbeff2..116c9e9637fe 100644 --- a/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts +++ b/packages/cloudflare/test/instrumentations/instrumentWorkerEntrypoint.test.ts @@ -412,6 +412,96 @@ describe('instrumentWorkerEntrypoint', () => { expect(events).toHaveLength(2); }); + it('shares the isolation scope with directly called instrumented methods', async () => { + const events: Event[] = []; + const waits: Promise[] = []; + const context = createMockExecutionContext(); + context.waitUntil = vi.fn(promise => { + waits.push(promise); + }); + const TestClass = class extends WorkerEntrypoint { + async outer() { + SentryCore.setTag('outer_tag', 'from-outer'); + + await this.inner(); + + SentryCore.captureMessage('outer message'); + } + + async inner() { + SentryCore.setTag('inner_tag', 'from-inner'); + SentryCore.setUser({ id: 'user-from-inner' }); + } + }; + const obj = Reflect.construct( + instrumentWorkerEntrypoint( + () => ({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + beforeSend(event) { + events.push(event); + return null; + }, + }), + TestClass as unknown as WorkerEntrypointConstructor, + ), + [context, {}], + ); + + await obj.outer(); + await Promise.all(waits); + + // `inner` is instrumented too, but it is reached from within `outer`'s invocation, so it must + // write to the scope `outer` already opened rather than fork one of its own. + expect(events[0]?.tags).toEqual(expect.objectContaining({ outer_tag: 'from-outer', inner_tag: 'from-inner' })); + expect(events[0]?.user).toEqual({ id: 'user-from-inner' }); + }); + + it('does not leak isolation scope data between consecutive invocations', async () => { + const events: Event[] = []; + const waits: Promise[] = []; + const context = createMockExecutionContext(); + context.waitUntil = vi.fn(promise => { + waits.push(promise); + }); + const TestClass = class extends WorkerEntrypoint { + async seed() { + SentryCore.setTag('seeded_tag', 'from-seeding-invocation'); + SentryCore.setUser({ id: 'user-from-seeding-invocation' }); + SentryCore.captureMessage('seed'); + } + + async probe() { + SentryCore.captureMessage('probe'); + } + }; + const obj = Reflect.construct( + instrumentWorkerEntrypoint( + () => ({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + beforeSend(event) { + events.push(event); + return null; + }, + }), + TestClass as unknown as WorkerEntrypointConstructor, + ), + [context, {}], + ); + + await obj.seed(); + await Promise.all(waits.splice(0)); + await obj.probe(); + await Promise.all(waits); + + // Guards the probe assertions against passing vacuously. + expect(events[0]?.tags).toEqual(expect.objectContaining({ seeded_tag: 'from-seeding-invocation' })); + expect(events[0]?.user).toEqual({ id: 'user-from-seeding-invocation' }); + + expect(events[1]?.message).toBe('probe'); + expect(events[1]?.tags?.seeded_tag).toBeUndefined(); + expect(events[1]?.user).toBeUndefined(); + }); + it('only excludes WorkerEntrypoint lifecycle methods from RPC instrumentation', async () => { const initAndBind = vi.spyOn(SentryCore, 'initAndBind'); const TestClass = class extends WorkerEntrypoint { diff --git a/packages/cloudflare/test/utils/invocationScope.test.ts b/packages/cloudflare/test/utils/invocationScope.test.ts new file mode 100644 index 000000000000..c13bdfe467ee --- /dev/null +++ b/packages/cloudflare/test/utils/invocationScope.test.ts @@ -0,0 +1,119 @@ +import { + getCurrentScope, + getGlobalScope, + getIsolationScope, + GLOBAL_OBJ, + type Scope, + setAsyncContextStrategy, +} from '@sentry/core'; +import { AsyncLocalStorage } from 'async_hooks'; +import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { withInvocationIsolationScope } from '../../src/utils/invocationScope'; + +describe('withInvocationIsolationScope()', () => { + beforeEach(() => { + getIsolationScope().clear(); + getCurrentScope().clear(); + getGlobalScope().clear(); + + (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; + setAsyncLocalStorageAsyncContextStrategy(); + }); + + it('forks the isolation scope at the entry point', () => { + const outerScope = getIsolationScope(); + + withInvocationIsolationScope(scope => { + expect(scope).not.toBe(outerScope); + expect(getIsolationScope()).toBe(scope); + }); + }); + + it('inherits data from the enclosing isolation scope', () => { + getIsolationScope().setTag('from-outer', 'yes'); + + withInvocationIsolationScope(scope => { + expect(scope.getScopeData().tags).toEqual({ 'from-outer': 'yes' }); + }); + }); + + it('does not leak data written inside the invocation to the enclosing scope', () => { + const outerScope = getIsolationScope(); + + withInvocationIsolationScope(scope => { + scope.setTag('from-invocation', 'yes'); + scope.setUser({ id: 'user-1' }); + }); + + expect(outerScope.getScopeData().tags).toEqual({}); + expect(outerScope.getScopeData().user).toEqual({}); + }); + + it('gives two sibling invocations independent scopes', () => { + const scopes: Scope[] = []; + + withInvocationIsolationScope(scope => { + scope.setTag('first', 'yes'); + scopes.push(scope); + }); + + withInvocationIsolationScope(scope => { + scopes.push(scope); + expect(scope.getScopeData().tags).toEqual({}); + }); + + expect(scopes[0]).not.toBe(scopes[1]); + }); + + it('reuses the invocation scope when reentrant', () => { + withInvocationIsolationScope(outer => { + withInvocationIsolationScope(inner => { + expect(inner).toBe(outer); + }); + }); + }); + + it('lets a reentrant call add to what the entry point set, and vice versa', () => { + withInvocationIsolationScope(outer => { + outer.setTag('outer', 'yes'); + + withInvocationIsolationScope(inner => { + expect(inner.getScopeData().tags).toEqual({ outer: 'yes' }); + inner.setTag('inner', 'yes'); + }); + + expect(outer.getScopeData().tags).toEqual({ outer: 'yes', inner: 'yes' }); + }); + }); + + it('treats an invocation following a reentrant one as a fresh entry point', () => { + withInvocationIsolationScope(outer => { + withInvocationIsolationScope(inner => { + inner.setTag('nested', 'yes'); + }); + + expect(outer.getScopeData().tags).toEqual({ nested: 'yes' }); + }); + + withInvocationIsolationScope(scope => { + expect(scope.getScopeData().tags).toEqual({}); + }); + }); + + it('degrades to a no-op fork under a non-forking strategy', () => { + // The core stack fallback never forks: `getIsolationScope()` always reports the shared default + // scope, and `withIsolationScope` reuses it. So the active isolation scope stays the shared one + // even inside the invocation — the computed clone is silently dropped by the strategy. Cloudflare + // always installs the AsyncLocalStorage strategy, so this branch is not hit in production. + setAsyncContextStrategy(undefined); + + const outerScope = getIsolationScope(); + + withInvocationIsolationScope(() => { + expect(getIsolationScope()).toBe(outerScope); + }); + + expect(getIsolationScope()).toBe(outerScope); + }); +}); diff --git a/packages/cloudflare/test/wrapMethodWithSentry.test.ts b/packages/cloudflare/test/wrapMethodWithSentry.test.ts index ea154816da09..45652c6f26e5 100644 --- a/packages/cloudflare/test/wrapMethodWithSentry.test.ts +++ b/packages/cloudflare/test/wrapMethodWithSentry.test.ts @@ -1,9 +1,11 @@ +import type { ExecutionContext } from '@cloudflare/workers-types'; import * as sentryCore from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { makeFlushLock } from '../src/flush'; import { getInstrumented } from '../src/instrument'; import * as sdk from '../src/sdk'; import { wrapMethodWithSentry } from '../src/wrapMethodWithSentry'; +import { resetSdk } from './testUtils'; const mocks = vi.hoisted(() => ({ flush: vi.fn().mockResolvedValue(true), @@ -24,31 +26,6 @@ vi.mock('../src/sdk', () => ({ init: vi.fn(() => createMockClient(true)), })); -// Mock sentry/core functions -vi.mock('@sentry/core', async importOriginal => { - const actual = await importOriginal(); - return { - ...actual, - getClient: vi.fn(), - withIsolationScope: vi.fn((callback: (scope: unknown) => unknown) => callback(createMockScope())), - withScope: vi.fn((callback: (scope: unknown) => unknown) => callback(createMockScope())), - startSpan: vi.fn((opts, callback) => callback(createMockSpan())), - startNewTrace: vi.fn(callback => callback()), - captureException: vi.fn(), - flush: vi.fn().mockResolvedValue(true), - getActiveSpan: vi.fn(), - }; -}); - -const mockedWithIsolationScope = vi.mocked(sentryCore.withIsolationScope); - -function createMockScope() { - return { - getClient: vi.fn(), - setClient: vi.fn(), - }; -} - function createMockSpan() { return { setAttribute: vi.fn(), @@ -88,6 +65,7 @@ describe('wrapMethodWithSentry', () => { afterEach(() => { vi.restoreAllMocks(); + resetSdk(); }); describe('basic wrapping', () => { @@ -137,7 +115,6 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); const result = wrapped(); - expect(handler).toHaveBeenCalled(); // Without storage, there's no linkPromise, so sync behavior is preserved expect(result).not.toBeInstanceOf(Promise); expect(result).toBe('sync-result'); @@ -253,6 +230,7 @@ describe('wrapMethodWithSentry', () => { describe('span creation', () => { it('creates span with spanName when provided', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); const handler = vi.fn().mockResolvedValue('result'); const options = { origin: 'auto.faas.cloudflare.durable_object', @@ -265,7 +243,7 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); await wrapped(); - expect(sentryCore.startSpan).toHaveBeenCalledWith( + expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ name: 'test-span', }), @@ -274,6 +252,7 @@ describe('wrapMethodWithSentry', () => { }); it('does not create span when spanName is not provided', async () => { + const startSpanSpy = vi.spyOn(sentryCore, 'startSpan'); const handler = vi.fn().mockResolvedValue('result'); const options = { origin: 'auto.faas.cloudflare.durable_object', @@ -285,12 +264,13 @@ describe('wrapMethodWithSentry', () => { await wrapped(); // startSpan should not be called when no spanName is provided - expect(sentryCore.startSpan).not.toHaveBeenCalled(); + expect(startSpanSpy).not.toHaveBeenCalled(); }); }); describe('error handling', () => { it('captures exceptions from sync methods', async () => { + const exceptionSpy = vi.spyOn(sentryCore, 'captureException'); const error = new Error('Test sync error'); const handler = vi.fn().mockImplementation(() => { throw error; @@ -304,7 +284,7 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); await expect(async () => wrapped()).rejects.toThrow('Test sync error'); - expect(sentryCore.captureException).toHaveBeenCalledWith(error, { + expect(exceptionSpy).toHaveBeenCalledWith(error, { mechanism: { type: 'auto.faas.cloudflare.durable_object', handled: false, @@ -313,6 +293,7 @@ describe('wrapMethodWithSentry', () => { }); it('captures exceptions from async methods', async () => { + const exceptionSpy = vi.spyOn(sentryCore, 'captureException'); const error = new Error('Test async error'); const handler = vi.fn().mockRejectedValue(error); const options = { @@ -324,7 +305,7 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); await expect(wrapped()).rejects.toThrow('Test async error'); - expect(sentryCore.captureException).toHaveBeenCalledWith(error, { + expect(exceptionSpy).toHaveBeenCalledWith(error, { mechanism: { type: 'auto.faas.cloudflare.durable_object', handled: false, @@ -334,23 +315,8 @@ describe('wrapMethodWithSentry', () => { }); describe('startNewTrace option', () => { - it('uses withIsolationScope when startNewTrace is true', async () => { - const handler = vi.fn().mockResolvedValue('result'); - const options = { - origin: 'auto.faas.cloudflare.durable_object', - options: {}, - context: createMockContext(), - startNewTrace: true, - spanName: 'alarm', - }; - - const wrapped = wrapMethodWithSentry(options, handler); - await wrapped(); - - expect(sentryCore.withIsolationScope).toHaveBeenCalled(); - }); - it('uses startNewTrace when startNewTrace is true and spanName is set', async () => { + const startNewTraceSpy = vi.spyOn(sentryCore, 'startNewTrace'); const handler = vi.fn().mockResolvedValue('result'); const options = { origin: 'auto.faas.cloudflare.durable_object', @@ -363,10 +329,11 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); await wrapped(); - expect(sentryCore.startNewTrace).toHaveBeenCalledWith(expect.any(Function)); + expect(startNewTraceSpy).toHaveBeenCalledWith(expect.any(Function)); }); it('does not use startNewTrace when startNewTrace is false', async () => { + const startNewTraceSpy = vi.spyOn(sentryCore, 'startNewTrace'); const handler = vi.fn().mockResolvedValue('result'); const options = { origin: 'auto.faas.cloudflare.durable_object', @@ -379,7 +346,7 @@ describe('wrapMethodWithSentry', () => { const wrapped = wrapMethodWithSentry(options, handler); await wrapped(); - expect(sentryCore.startNewTrace).not.toHaveBeenCalled(); + expect(startNewTraceSpy).not.toHaveBeenCalled(); }); }); @@ -427,7 +394,7 @@ describe('wrapMethodWithSentry', () => { const mockStorage = { kv: mockKv }; const mockSpan = createMockSpan(); - vi.mocked(sentryCore.startSpan).mockImplementation((opts, callback) => callback(mockSpan as any)); + vi.spyOn(sentryCore, 'startSpan').mockImplementation((opts, callback) => callback(mockSpan as any)); const context = { waitUntil: vi.fn(), @@ -461,7 +428,7 @@ describe('wrapMethodWithSentry', () => { }); it('stores span context after execution when startNewTrace is true', async () => { - vi.mocked(sentryCore.getActiveSpan).mockReturnValue({ + vi.spyOn(sentryCore, 'getActiveSpan').mockReturnValue({ spanContext: vi.fn().mockReturnValue({ traceId: 'current-trace-id-123456789012345678', spanId: 'current-span-id', @@ -495,7 +462,7 @@ describe('wrapMethodWithSentry', () => { }); it('does not store span context when startNewTrace is false', async () => { - vi.mocked(sentryCore.getActiveSpan).mockReturnValue({ + vi.spyOn(sentryCore, 'getActiveSpan').mockReturnValue({ spanContext: vi.fn().mockReturnValue({ traceId: 'current-trace-id-123456789012345678', spanId: 'current-span-id', @@ -636,7 +603,7 @@ describe('wrapMethodWithSentry', () => { it('creates a new client when scope has no client', async () => { const scope = new sentryCore.Scope(); - mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope))); + vi.spyOn(sentryCore, 'getIsolationScope').mockReturnValue(scope); const spyClient = vi.spyOn(scope, 'setClient'); const handler = vi.fn().mockResolvedValue('result'); @@ -670,7 +637,7 @@ describe('wrapMethodWithSentry', () => { const scope = new sentryCore.Scope(); scope.setClient(disposedClient); - mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope))); + vi.spyOn(sentryCore, 'getIsolationScope').mockReturnValue(scope); const spyClient = vi.spyOn(scope, 'setClient'); const handler = vi.fn().mockResolvedValue('result'); @@ -703,7 +670,7 @@ describe('wrapMethodWithSentry', () => { const scope = new sentryCore.Scope(); scope.setClient(validClient); - mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope))); + vi.spyOn(sentryCore, 'getIsolationScope').mockReturnValue(scope); vi.mocked(sdk.init).mockClear(); const spyClient = vi.spyOn(scope, 'setClient'); @@ -731,6 +698,7 @@ describe('wrapMethodWithSentry waitUntil teardown (hibernation regression)', () afterEach(() => { vi.restoreAllMocks(); + resetSdk(); }); // Regression for #22328