From fe76a284276c2079d043e4d510323cc8ba192a9c Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 28 Jul 2025 11:11:01 +0200 Subject: [PATCH 1/3] fix(cloudflare): Avoid turning DurableObject sync methods into async From bb5d3e90f7eaa860f82a400ff7396633ed8cbeec Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 28 Jul 2025 11:11:16 +0200 Subject: [PATCH 2/3] Add failing test --- .../cloudflare/test/durableobject.test.ts | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index b627c4051b41..2add5dde9343 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -8,6 +8,7 @@ describe('instrumentDurableObjectWithSentry', () => { afterEach(() => { vi.restoreAllMocks(); }); + it('Generic functionality', () => { const options = vi.fn(); const instrumented = instrumentDurableObjectWithSentry(options, vi.fn()); @@ -15,13 +16,35 @@ describe('instrumentDurableObjectWithSentry', () => { expect(() => Reflect.construct(instrumented, [])).not.toThrow(); expect(options).toHaveBeenCalledOnce(); }); - it('Instruments prototype methods and defines implementation in the object', () => { + + it('Instruments sync prototype methods and defines implementation in the object', () => { const testClass = class { - method() {} + method() { + return 'sync-result'; + } }; const obj = Reflect.construct(instrumentDurableObjectWithSentry(vi.fn(), testClass as any), []) as any; expect(obj.method).toBe(obj.method); + + const result = obj.method(); + expect(result).not.toBeInstanceOf(Promise); + expect(result).toEqual('sync-result'); }); + + it('Instruments async prototype methods and returns a promise', async () => { + const testClass = class { + async asyncMethod() { + return 'async-result'; + } + }; + const obj = Reflect.construct(instrumentDurableObjectWithSentry(vi.fn(), testClass as any), []) as any; + expect(obj.asyncMethod).toBe(obj.asyncMethod); + + const result = obj.asyncMethod(); + expect(result).toBeInstanceOf(Promise); + expect(await result).toBe('async-result'); + }); + it('Instruments prototype methods without "sticking" to the options', () => { const initCore = vi.spyOn(SentryCore, 'initAndBind'); vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined); @@ -41,6 +64,7 @@ describe('instrumentDurableObjectWithSentry', () => { expect(initCore).nthCalledWith(1, expect.any(Function), expect.objectContaining({ orgId: 1 })); expect(initCore).nthCalledWith(2, expect.any(Function), expect.objectContaining({ orgId: 2 })); }); + it('All available durable object methods are instrumented', () => { const testClass = class { propertyFunction = vi.fn(); @@ -72,6 +96,7 @@ describe('instrumentDurableObjectWithSentry', () => { expect(isInstrumented((obj as any)[method_name]), `Method ${method_name} is instrumented`).toBeTruthy(); } }); + it('flush performs after all waitUntil promises are finished', async () => { vi.useFakeTimers(); onTestFinished(() => { From 681959d1210681798ada568e4eb1d56da14387d5 Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 28 Jul 2025 12:10:05 +0200 Subject: [PATCH 3/3] Avoid turning wrapped sync methods into async --- packages/cloudflare/src/durableobject.ts | 65 ++++++++++++++++++++---- 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/packages/cloudflare/src/durableobject.ts b/packages/cloudflare/src/durableobject.ts index c9de8763750c..4efaf33c9b1c 100644 --- a/packages/cloudflare/src/durableobject.ts +++ b/packages/cloudflare/src/durableobject.ts @@ -1,8 +1,10 @@ /* eslint-disable @typescript-eslint/unbound-method */ import { + type Scope, captureException, flush, getClient, + isThenable, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startSpan, @@ -46,7 +48,8 @@ function wrapMethodWithSentry( const currentClient = getClient(); // if a client is already set, use withScope, otherwise use withIsolationScope const sentryWithScope = currentClient ? withScope : withIsolationScope; - return sentryWithScope(async scope => { + + const wrappedFunction = (scope: Scope): unknown => { // In certain situations, the passed context can become undefined. // For example, for Astro while prerendering pages at build time. // see: https://github.com/getsentry/sentry-javascript/issues/13217 @@ -65,7 +68,29 @@ function wrapMethodWithSentry( if (callback) { callback(...args); } - return await Reflect.apply(target, thisArg, args); + const result = Reflect.apply(target, thisArg, args); + + if (isThenable(result)) { + return result.then( + (res: unknown) => { + waitUntil?.(flush(2000)); + return res; + }, + (e: unknown) => { + captureException(e, { + mechanism: { + type: 'cloudflare_durableobject', + handled: false, + }, + }); + waitUntil?.(flush(2000)); + throw e; + }, + ); + } else { + waitUntil?.(flush(2000)); + return result; + } } catch (e) { captureException(e, { mechanism: { @@ -73,9 +98,8 @@ function wrapMethodWithSentry( handled: false, }, }); - throw e; - } finally { waitUntil?.(flush(2000)); + throw e; } } @@ -87,9 +111,31 @@ function wrapMethodWithSentry( : {}; // Only create these spans if they have a parent span. - return startSpan({ name: wrapperOptions.spanName, attributes, onlyIfParent: true }, async () => { + return startSpan({ name: wrapperOptions.spanName, attributes, onlyIfParent: true }, () => { try { - return await Reflect.apply(target, thisArg, args); + const result = Reflect.apply(target, thisArg, args); + + if (isThenable(result)) { + return result.then( + (res: unknown) => { + waitUntil?.(flush(2000)); + return res; + }, + (e: unknown) => { + captureException(e, { + mechanism: { + type: 'cloudflare_durableobject', + handled: false, + }, + }); + waitUntil?.(flush(2000)); + throw e; + }, + ); + } else { + waitUntil?.(flush(2000)); + return result; + } } catch (e) { captureException(e, { mechanism: { @@ -97,12 +143,13 @@ function wrapMethodWithSentry( handled: false, }, }); - throw e; - } finally { waitUntil?.(flush(2000)); + throw e; } }); - }); + }; + + return sentryWithScope(wrappedFunction); }, }); }