Skip to content

Commit f502420

Browse files
authored
fix(v9/cloudflare): Avoid turning DurableObject sync methods into async (#17187)
Backport of #17184
1 parent 2524460 commit f502420

File tree

2 files changed

+83
-11
lines changed

2 files changed

+83
-11
lines changed

packages/cloudflare/src/durableobject.ts

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
/* eslint-disable @typescript-eslint/unbound-method */
22
import {
3+
type Scope,
34
captureException,
45
flush,
56
getClient,
7+
isThenable,
68
SEMANTIC_ATTRIBUTE_SENTRY_OP,
79
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
810
startSpan,
@@ -46,7 +48,8 @@ function wrapMethodWithSentry<T extends OriginalMethod>(
4648
const currentClient = getClient();
4749
// if a client is already set, use withScope, otherwise use withIsolationScope
4850
const sentryWithScope = currentClient ? withScope : withIsolationScope;
49-
return sentryWithScope(async scope => {
51+
52+
const wrappedFunction = (scope: Scope): unknown => {
5053
// In certain situations, the passed context can become undefined.
5154
// For example, for Astro while prerendering pages at build time.
5255
// see: https://github.com/getsentry/sentry-javascript/issues/13217
@@ -65,17 +68,38 @@ function wrapMethodWithSentry<T extends OriginalMethod>(
6568
if (callback) {
6669
callback(...args);
6770
}
68-
return await Reflect.apply(target, thisArg, args);
71+
const result = Reflect.apply(target, thisArg, args);
72+
73+
if (isThenable(result)) {
74+
return result.then(
75+
(res: unknown) => {
76+
waitUntil?.(flush(2000));
77+
return res;
78+
},
79+
(e: unknown) => {
80+
captureException(e, {
81+
mechanism: {
82+
type: 'cloudflare_durableobject',
83+
handled: false,
84+
},
85+
});
86+
waitUntil?.(flush(2000));
87+
throw e;
88+
},
89+
);
90+
} else {
91+
waitUntil?.(flush(2000));
92+
return result;
93+
}
6994
} catch (e) {
7095
captureException(e, {
7196
mechanism: {
7297
type: 'cloudflare_durableobject',
7398
handled: false,
7499
},
75100
});
76-
throw e;
77-
} finally {
78101
waitUntil?.(flush(2000));
102+
throw e;
79103
}
80104
}
81105

@@ -87,22 +111,45 @@ function wrapMethodWithSentry<T extends OriginalMethod>(
87111
: {};
88112

89113
// Only create these spans if they have a parent span.
90-
return startSpan({ name: wrapperOptions.spanName, attributes, onlyIfParent: true }, async () => {
114+
return startSpan({ name: wrapperOptions.spanName, attributes, onlyIfParent: true }, () => {
91115
try {
92-
return await Reflect.apply(target, thisArg, args);
116+
const result = Reflect.apply(target, thisArg, args);
117+
118+
if (isThenable(result)) {
119+
return result.then(
120+
(res: unknown) => {
121+
waitUntil?.(flush(2000));
122+
return res;
123+
},
124+
(e: unknown) => {
125+
captureException(e, {
126+
mechanism: {
127+
type: 'cloudflare_durableobject',
128+
handled: false,
129+
},
130+
});
131+
waitUntil?.(flush(2000));
132+
throw e;
133+
},
134+
);
135+
} else {
136+
waitUntil?.(flush(2000));
137+
return result;
138+
}
93139
} catch (e) {
94140
captureException(e, {
95141
mechanism: {
96142
type: 'cloudflare_durableobject',
97143
handled: false,
98144
},
99145
});
100-
throw e;
101-
} finally {
102146
waitUntil?.(flush(2000));
147+
throw e;
103148
}
104149
});
105-
});
150+
};
151+
152+
return sentryWithScope(wrappedFunction);
106153
},
107154
});
108155
}

packages/cloudflare/test/durableobject.test.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,43 @@ describe('instrumentDurableObjectWithSentry', () => {
88
afterEach(() => {
99
vi.restoreAllMocks();
1010
});
11+
1112
it('Generic functionality', () => {
1213
const options = vi.fn();
1314
const instrumented = instrumentDurableObjectWithSentry(options, vi.fn());
1415
expect(instrumented).toBeTypeOf('function');
1516
expect(() => Reflect.construct(instrumented, [])).not.toThrow();
1617
expect(options).toHaveBeenCalledOnce();
1718
});
18-
it('Instruments prototype methods and defines implementation in the object', () => {
19+
20+
it('Instruments sync prototype methods and defines implementation in the object', () => {
1921
const testClass = class {
20-
method() {}
22+
method() {
23+
return 'sync-result';
24+
}
2125
};
2226
const obj = Reflect.construct(instrumentDurableObjectWithSentry(vi.fn(), testClass as any), []) as any;
2327
expect(obj.method).toBe(obj.method);
28+
29+
const result = obj.method();
30+
expect(result).not.toBeInstanceOf(Promise);
31+
expect(result).toEqual('sync-result');
2432
});
33+
34+
it('Instruments async prototype methods and returns a promise', async () => {
35+
const testClass = class {
36+
async asyncMethod() {
37+
return 'async-result';
38+
}
39+
};
40+
const obj = Reflect.construct(instrumentDurableObjectWithSentry(vi.fn(), testClass as any), []) as any;
41+
expect(obj.asyncMethod).toBe(obj.asyncMethod);
42+
43+
const result = obj.asyncMethod();
44+
expect(result).toBeInstanceOf(Promise);
45+
expect(await result).toBe('async-result');
46+
});
47+
2548
it('Instruments prototype methods without "sticking" to the options', () => {
2649
const initCore = vi.spyOn(SentryCore, 'initAndBind');
2750
vi.spyOn(SentryCore, 'getClient').mockReturnValue(undefined);
@@ -41,6 +64,7 @@ describe('instrumentDurableObjectWithSentry', () => {
4164
expect(initCore).nthCalledWith(1, expect.any(Function), expect.objectContaining({ orgId: 1 }));
4265
expect(initCore).nthCalledWith(2, expect.any(Function), expect.objectContaining({ orgId: 2 }));
4366
});
67+
4468
it('All available durable object methods are instrumented', () => {
4569
const testClass = class {
4670
propertyFunction = vi.fn();
@@ -72,6 +96,7 @@ describe('instrumentDurableObjectWithSentry', () => {
7296
expect(isInstrumented((obj as any)[method_name]), `Method ${method_name} is instrumented`).toBeTruthy();
7397
}
7498
});
99+
75100
it('flush performs after all waitUntil promises are finished', async () => {
76101
vi.useFakeTimers();
77102
onTestFinished(() => {

0 commit comments

Comments
 (0)