Skip to content

Commit 625ab92

Browse files
JPeer264claude
andcommitted
fix(cloudflare): Don't treat DO constructor work as incoming RPC calls
The RPC prototype wrapper treats a call without trace data on the default isolation scope as an incoming RPC call, starts a new trace for it and captures its errors as unhandled. The SDK ran Durable Object constructors on the default isolation scope, so async work the constructor started, such as a blockConcurrencyWhile callback, reached the wrappers with that scope. Its calls to the instance's own methods were captured as untraced RPC entries, including errors the instance caught itself. Run the constructor on a forked construction scope, which that async work inherits. Instrumented handlers called from it still fork their own scope, and the construction scope carries no invocation state, so captures made directly in that work are delivered as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 48c8b87 commit 625ab92

8 files changed

Lines changed: 292 additions & 9 deletions

File tree

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import * as Sentry from '@sentry/cloudflare';
2+
import { DurableObject } from 'cloudflare:workers';
3+
4+
interface Env {
5+
SENTRY_DSN: string;
6+
BLOCK_CONCURRENCY_DURABLE_OBJECT: DurableObjectNamespace<BlockConcurrencyDurableObject>;
7+
ASYNC_METHOD_DURABLE_OBJECT: DurableObjectNamespace<AsyncMethodDurableObject>;
8+
}
9+
10+
export class BlockConcurrencyDurableObject extends DurableObject<Env> {
11+
private failure?: Error;
12+
13+
constructor(ctx: DurableObjectState, env: Env) {
14+
super(ctx, env);
15+
16+
// The callback runs after the constructor has returned, so after the SDK has wrapped `init`.
17+
void ctx.blockConcurrencyWhile(async () => {
18+
try {
19+
this.init();
20+
} catch (error) {
21+
this.failure = error as Error;
22+
}
23+
});
24+
}
25+
26+
init(): void {
27+
throw new Error('Init failed');
28+
}
29+
30+
ping(): string {
31+
const status = this.failure ? 'degraded' : 'ok';
32+
Sentry.captureMessage(`block-concurrency-while ping: ${status}`);
33+
return status;
34+
}
35+
}
36+
37+
export class AsyncMethodDurableObject extends DurableObject<Env> {
38+
private failure?: Error;
39+
private readonly loaded: Promise<void>;
40+
41+
constructor(ctx: DurableObjectState, env: Env) {
42+
super(ctx, env);
43+
44+
this.loaded = this.load();
45+
}
46+
47+
async load(): Promise<void> {
48+
// Everything after the first `await` runs after the constructor has returned, so after the SDK
49+
// has wrapped `init`.
50+
await this.ctx.storage.get('config');
51+
52+
try {
53+
this.init();
54+
} catch (error) {
55+
this.failure = error as Error;
56+
}
57+
}
58+
59+
init(): void {
60+
throw new Error('Init failed');
61+
}
62+
63+
async ping(): Promise<string> {
64+
await this.loaded;
65+
const status = this.failure ? 'degraded' : 'ok';
66+
Sentry.captureMessage(`async-method ping: ${status}`);
67+
return status;
68+
}
69+
}
70+
71+
export default {
72+
async fetch(request: Request, env: Env): Promise<Response> {
73+
const url = new URL(request.url);
74+
75+
if (url.pathname === '/block-concurrency-while') {
76+
const namespace = env.BLOCK_CONCURRENCY_DURABLE_OBJECT;
77+
return new Response(await namespace.get(namespace.idFromName('test')).ping());
78+
}
79+
80+
if (url.pathname === '/async-method') {
81+
const namespace = env.ASYNC_METHOD_DURABLE_OBJECT;
82+
return new Response(await namespace.get(namespace.idFromName('test')).ping());
83+
}
84+
85+
return new Response('Not found', { status: 404 });
86+
},
87+
} satisfies ExportedHandler<Env>;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { defineCloudflareOptions } from '@sentry/cloudflare';
2+
3+
export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({
4+
dsn: env.SENTRY_DSN,
5+
tracesSampleRate: 1.0,
6+
}));
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import type { Event, SerializedStreamedSpanContainer } from '@sentry/core';
2+
import { SENTRY_OP, SENTRY_ORIGIN, URL_PATH } from '@sentry/conventions/attributes';
3+
import { expect, it } from 'vitest';
4+
import { createRunner } from '../../../runner';
5+
6+
// Every envelope of both requests is expected, and `failOnUnexpected` fails the test on any other one.
7+
// All of them are sent after the constructor work ran, so an error or span from that work would arrive
8+
// before the runner completes.
9+
it('does not capture errors a Durable Object catches in work its constructor started', async ({ signal }) => {
10+
const runner = createRunner(__dirname)
11+
.unordered()
12+
.failOnUnexpected()
13+
.expect(envelope => {
14+
expect(envelope[1][0][0].type).toBe('event');
15+
expect((envelope[1][0][1] as Event).message).toBe('block-concurrency-while ping: degraded');
16+
})
17+
.expect(envelope => {
18+
expect(envelope[1][0][0].type).toBe('event');
19+
expect((envelope[1][0][1] as Event).message).toBe('async-method ping: degraded');
20+
})
21+
.expectN(2, envelope => {
22+
expect(envelope[1][0][0].type).toBe('span');
23+
const container = envelope[1][0][1] as SerializedStreamedSpanContainer;
24+
expect(container.items.map(item => item.name)).toEqual(['ping']);
25+
expect(container.items[0]?.attributes[SENTRY_OP]?.value).toBe('rpc');
26+
expect(container.items[0]?.attributes[SENTRY_ORIGIN]?.value).toBe('auto.faas.cloudflare.durable_object');
27+
})
28+
.expect(envelope => {
29+
expect(envelope[1][0][0].type).toBe('span');
30+
const container = envelope[1][0][1] as SerializedStreamedSpanContainer;
31+
expect(container.items.map(item => item.attributes[URL_PATH]?.value)).toEqual(['/block-concurrency-while']);
32+
expect(container.items[0]?.attributes[SENTRY_OP]?.value).toBe('http.server');
33+
})
34+
.expect(envelope => {
35+
expect(envelope[1][0][0].type).toBe('span');
36+
const container = envelope[1][0][1] as SerializedStreamedSpanContainer;
37+
expect(container.items.map(item => item.attributes[URL_PATH]?.value)).toEqual(['/async-method']);
38+
expect(container.items[0]?.attributes[SENTRY_OP]?.value).toBe('http.server');
39+
})
40+
.start(signal);
41+
42+
const blockConcurrencyResponse = await runner.makeRequest<string>('get', '/block-concurrency-while');
43+
const asyncMethodResponse = await runner.makeRequest<string>('get', '/async-method');
44+
45+
expect(blockConcurrencyResponse).toBe('degraded');
46+
expect(asyncMethodResponse).toBe('degraded');
47+
await runner.completed();
48+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import { cloudflare } from '@cloudflare/vite-plugin';
2+
import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite';
3+
import { defineConfig } from 'vite';
4+
5+
export default defineConfig({
6+
plugins: [cloudflare(), sentryCloudflareVitePlugin()],
7+
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"$schema": "../../../node_modules/wrangler/config-schema.json",
3+
"name": "durableobject-constructor-async-work-worker",
4+
"main": "index.ts",
5+
"compatibility_date": "2025-06-17",
6+
"compatibility_flags": ["nodejs_compat"],
7+
"durable_objects": {
8+
"bindings": [
9+
{ "name": "BLOCK_CONCURRENCY_DURABLE_OBJECT", "class_name": "BlockConcurrencyDurableObject" },
10+
{ "name": "ASYNC_METHOD_DURABLE_OBJECT", "class_name": "AsyncMethodDurableObject" },
11+
],
12+
},
13+
"migrations": [{ "tag": "v1", "new_sqlite_classes": ["BlockConcurrencyDurableObject", "AsyncMethodDurableObject"] }],
14+
}

‎packages/cloudflare/src/durableobject.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { instrumentDurableObjectHandlers } from './instrumentations/instrumentDu
1010
import { instrumentEnv } from './instrumentations/worker/instrumentEnv';
1111
import { getFinalOptions } from './options';
1212
import { instrumentContext } from './utils/instrumentContext';
13+
import { withConstructionIsolationScope } from './utils/invocationScope';
1314
import { hasRpcMeta } from './utils/rpcMeta';
1415
import { instrumentCloudflareAgent } from './instrumentations/agents';
1516
import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types';
@@ -67,7 +68,9 @@ export function constructInstrumentedDurableObject<E, T extends DurableObject<E>
6768
// Pass `newTarget` so that subclasses of the instrumented class (e.g. the wrapper classes
6869
// created by wrangler's local dev tooling or `@cloudflare/vitest-pool-workers`) keep their
6970
// own prototype — otherwise subclass methods disappear and `instanceof` checks break.
70-
const obj = Reflect.construct(target, [context, instrumentedEnv], newTarget) as T;
71+
const obj = withConstructionIsolationScope(
72+
() => Reflect.construct(target, [context, instrumentedEnv], newTarget) as T,
73+
);
7174

7275
const frameworkManagedMethods = resolveFrameworkManagedMethods(
7376
prototype,
@@ -298,9 +301,10 @@ function createRpcPrototypeWrapper(methodName: string, originalMethod: Unchecked
298301
const wrapper = function (this: unknown, ...args: unknown[]): unknown {
299302
const traced = hasRpcMeta(args);
300303

301-
// workerd dispatches an incoming RPC call outside any async context, so a call made while an
302-
// invocation is already in flight comes from the instance itself (`this.helper()` inside
303-
// `fetch`, `alarm` or another RPC method). Check that before touching per-instance state.
304+
// workerd dispatches an incoming RPC call outside any async context, so a call made on any other
305+
// isolation scope comes from the instance itself (`this.helper()` inside `fetch`, `alarm`,
306+
// another RPC method or async work the constructor started). Check that before touching
307+
// per-instance state.
304308
if (!traced && getIsolationScope() !== getDefaultIsolationScope()) {
305309
return Reflect.apply(originalMethod, this, args);
306310
}

‎packages/cloudflare/src/utils/invocationScope.ts‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,27 @@ import { getDefaultIsolationScope, getIsolationScope, type Scope, withIsolationS
22
import type { ExecutionContextCompat } from '../executionContext';
33
import { setInvocationState } from './invocationContext';
44

5+
const constructionScopes = new WeakSet<Scope>();
6+
7+
/**
8+
* Runs a Durable Object constructor on a forked isolation scope.
9+
*
10+
* Async work the constructor starts, such as a `blockConcurrencyWhile` callback, runs after the RPC
11+
* wrappers are installed and inherits this scope, so the calls it makes to the instance's own methods
12+
* are not treated as incoming RPC calls. The scope is not an invocation: it has no invocation state,
13+
* and an instrumented handler that this work calls still forks a scope of its own.
14+
*/
15+
export function withConstructionIsolationScope<T>(callback: () => T): T {
16+
if (getIsolationScope() !== getDefaultIsolationScope()) {
17+
return callback();
18+
}
19+
20+
return withIsolationScope(scope => {
21+
constructionScopes.add(scope);
22+
return callback();
23+
});
24+
}
25+
526
/**
627
* Runs `callback` on the isolation scope for the current invocation.
728
*
@@ -19,14 +40,15 @@ import { setInvocationState } from './invocationContext';
1940
*
2041
* The AsyncLocalStorage strategy hands the default isolation scope back whenever no invocation is in
2142
* flight, and a forked one while inside `withIsolationScope`. Reference-comparing against the default
22-
* is therefore enough to tell the two cases apart. The stack fallback does not fork, so it reports the
23-
* default scope even inside an invocation; there the fork degrades to a no-op, which the stack strategy
24-
* tolerates. This matches the approach used by `patchEventHandler` in Nuxt.
43+
* and the construction scopes (see {@link withConstructionIsolationScope}) is therefore enough to tell
44+
* the two cases apart. The stack fallback does not fork, so it reports the default scope even inside an
45+
* invocation; there the fork degrades to a no-op, which the stack strategy tolerates. This matches the
46+
* approach used by `patchEventHandler` in Nuxt.
2547
*/
2648
export function withInvocationIsolationScope<T>(callback: (scope: Scope) => T, context?: ExecutionContextCompat): T {
2749
const isolationScope = getIsolationScope();
2850

29-
const isEntryPoint = isolationScope === getDefaultIsolationScope();
51+
const isEntryPoint = isolationScope === getDefaultIsolationScope() || constructionScopes.has(isolationScope);
3052
const newIsolationScope = isEntryPoint ? isolationScope.clone() : isolationScope;
3153

3254
if (isEntryPoint) {

‎packages/cloudflare/test/durableobject.test.ts‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,62 @@ describe('instrumentDurableObjectWithSentry', () => {
216216
expect(events[2]?.user).toBeUndefined();
217217
});
218218

219+
// Work the constructor starts shares one scope, so a handler it calls must not reuse that scope.
220+
it('Built-in handlers called from work the constructor started each get their own isolation scope', async () => {
221+
const events: Event[] = [];
222+
const waits: Promise<unknown>[] = [];
223+
const mockContext = {
224+
waitUntil: vi.fn((promise: Promise<unknown>) => {
225+
waits.push(promise);
226+
}),
227+
blockConcurrencyWhile: (callback: () => Promise<unknown>) => Promise.resolve().then(callback),
228+
} as any;
229+
230+
const testClass = class {
231+
initialized: Promise<void>;
232+
233+
constructor(ctx: { blockConcurrencyWhile(callback: () => Promise<void>): Promise<void> }) {
234+
this.initialized = ctx.blockConcurrencyWhile(async () => {
235+
await this.webSocketMessage({}, 'seed');
236+
await this.webSocketMessage({}, 'probe');
237+
});
238+
}
239+
240+
webSocketMessage(_ws: unknown, message: string) {
241+
if (message === 'seed') {
242+
SentryCore.setTag('seeded_tag', 'from-seeding-message');
243+
SentryCore.setUser({ id: 'user-from-seeding-message' });
244+
}
245+
246+
SentryCore.captureMessage(message);
247+
}
248+
};
249+
const obj = Reflect.construct(
250+
instrumentDurableObjectWithSentry(
251+
() => ({
252+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
253+
beforeSend(event: Event) {
254+
events.push(event);
255+
return null;
256+
},
257+
}),
258+
testClass as any,
259+
),
260+
[mockContext, {} as any],
261+
);
262+
263+
await obj.initialized;
264+
await Promise.all(waits);
265+
266+
expect(events.map(event => event.message)).toEqual(['seed', 'probe']);
267+
// Guards the assertions below against passing vacuously.
268+
expect(events[0]?.tags?.seeded_tag).toBe('from-seeding-message');
269+
expect(events[0]?.user).toEqual({ id: 'user-from-seeding-message' });
270+
271+
expect(events[1]?.tags?.seeded_tag).toBeUndefined();
272+
expect(events[1]?.user).toBeUndefined();
273+
});
274+
219275
it('Built-in durable object methods are always instrumented', () => {
220276
const testClass = class {
221277
fetch() {}
@@ -654,6 +710,9 @@ describe('instrumentDurableObjectWithSentry', () => {
654710
const waitUntil = vi.fn((promise: Promise<unknown>) => {
655711
waits.push(promise);
656712
});
713+
// Like workerd, runs the callback after the constructor returns, in the async context of the caller.
714+
const blockConcurrencyWhile = (callback: () => Promise<unknown>): Promise<unknown> =>
715+
Promise.resolve().then(callback);
657716

658717
const instrumented = instrumentDurableObjectWithSentry(
659718
() => ({
@@ -675,7 +734,7 @@ describe('instrumentDurableObjectWithSentry', () => {
675734
}),
676735
testClass as any,
677736
);
678-
const obj = Reflect.construct(instrumented, [{ waitUntil }, {}]) as InstanceType<C>;
737+
const obj = Reflect.construct(instrumented, [{ waitUntil, blockConcurrencyWhile }, {}]) as InstanceType<C>;
679738
const settle = async (): Promise<void> => {
680739
while (waits.length) {
681740
await Promise.all(waits.splice(0));
@@ -769,6 +828,42 @@ describe('instrumentDurableObjectWithSentry', () => {
769828
expect(waitUntil).toHaveBeenCalledOnce();
770829
});
771830

831+
it('does not instrument calls from a blockConcurrencyWhile callback the constructor started', async () => {
832+
const { obj, events, waitUntil, settle } = setup(
833+
class {
834+
failure?: Error;
835+
initialized: Promise<void>;
836+
837+
constructor(ctx: { blockConcurrencyWhile(callback: () => Promise<void>): Promise<void> }) {
838+
this.initialized = ctx.blockConcurrencyWhile(async () => {
839+
try {
840+
this.init();
841+
} catch (error) {
842+
this.failure = error as Error;
843+
}
844+
});
845+
}
846+
847+
init(): void {
848+
throw new Error('Init failed');
849+
}
850+
851+
async ping(): Promise<string> {
852+
const status = this.failure ? 'degraded' : 'ok';
853+
SentryCore.captureMessage(`ping: ${status}`);
854+
return status;
855+
}
856+
},
857+
);
858+
859+
await obj.initialized;
860+
await expect(obj.ping()).resolves.toBe('degraded');
861+
await settle();
862+
863+
expect(events.map(event => event.message)).toEqual(['ping: degraded']);
864+
expect(waitUntil).toHaveBeenCalledOnce();
865+
});
866+
772867
it('continues the caller trace when the call carries trace metadata', async () => {
773868
const { obj, events, transactions, settle } = setup(
774869
class {

0 commit comments

Comments
 (0)