From edb2f4fada69c357135b112c248c891e07d552c7 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Thu, 30 Jul 2026 09:24:22 +0200 Subject: [PATCH 1/2] fix(cloudflare): Fork the isolation scope for Durable Object methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `setUser`/`setTag` write to the isolation scope, and a Durable Object keeps that scope across invocations. Methods only forked the current scope while no client was bound — but disposing a client at the invocation boundary does not unbind it, so from the second invocation onward the still-assigned client made every entry point look reentrant and skip its fork. Data from one invocation thus reappeared on the next, and a user identity could attach itself to an unrelated event. An instrumented handler is either an invocation's entry point or reentrant (a DO method calling its own `fetch`, an RPC method reaching a sibling). Only the entry point may fork; a bound client can't distinguish the two, so `withInvocationIsolationScope` records the fact directly as a marker in SDK processing metadata (stripped before send). The stack fallback doesn't clone, so its scope is left unmarked rather than making every later entry point look reentrant. Forking loses nothing — it clones, inheriting enclosing request data — and matches how the Worker `fetch` path already behaves. Covered by integration tests against a real Durable Object (consecutive invocations, a nested direct call, a nested call onto the instrumented `fetch`) and unit tests for the reentrancy logic plus the `instrumentWorkerEntrypoint` RPC and `webSocketMessage`/`alarm` consumers. Co-authored-by: Cursor Co-Authored-By: Claude Opus 5 --- .../suites/durableobject-scope/index.ts | 115 +++++++++++++++++ .../suites/durableobject-scope/test.ts | 77 ++++++++++++ .../suites/durableobject-scope/wrangler.jsonc | 15 +++ packages/cloudflare/src/request.ts | 4 +- .../cloudflare/src/utils/invocationScope.ts | 30 +++++ .../cloudflare/src/wrapMethodWithSentry.ts | 11 +- .../cloudflare/test/durableobject.test.ts | 63 ++++++++++ .../instrumentWorkerEntrypoint.test.ts | 90 +++++++++++++ .../test/utils/invocationScope.test.ts | 119 ++++++++++++++++++ .../test/wrapMethodWithSentry.test.ts | 76 ++++------- 10 files changed, 535 insertions(+), 65 deletions(-) create mode 100644 dev-packages/cloudflare-integration-tests/suites/durableobject-scope/index.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/durableobject-scope/wrangler.jsonc create mode 100644 packages/cloudflare/src/utils/invocationScope.ts create mode 100644 packages/cloudflare/test/utils/invocationScope.test.ts 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..858affadf671 --- /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('cacheClient: false - 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 From 91ea0beec680199624d46fe18f9a5c64c70c5384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Peer=20St=C3=B6cklmair?= Date: Tue, 4 Aug 2026 11:23:12 +0200 Subject: [PATCH 2/2] Simplify test description for isolation scopes --- .../suites/durableobject-scope/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts index 858affadf671..5da3d134d60a 100644 --- a/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/durableobject-scope/test.ts @@ -2,7 +2,7 @@ import type { Envelope, Event } from '@sentry/core'; import { expect, it } from 'vitest'; import { createRunner } from '../../runner'; -it('cacheClient: false - two consecutive invocations get different isolation scopes', async ({ signal }) => { +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) => {