From a73ee641e3ad808d8ea9b6b1eb33dcda5e9b53f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Peer=20St=C3=B6cklmair?= Date: Mon, 31 Aug 2026 15:40:02 +0300 Subject: [PATCH] fix(v10/cloudflare): Instrument Durable Object handlers installed as read-only properties Backport of: #23759 --- .../cloudflare-agent/package.json | 5 +- packages/cloudflare/src/durableobject.ts | 84 +-------- .../instrumentDurableObjectHandlers.ts | 161 ++++++++++++++++++ .../cloudflare/test/durableobject.test.ts | 51 ++++++ 4 files changed, 219 insertions(+), 82 deletions(-) create mode 100644 packages/cloudflare/src/instrumentations/instrumentDurableObjectHandlers.ts diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-agent/package.json b/dev-packages/e2e-tests/test-applications/cloudflare-agent/package.json index 21ed3f3a9b8b..5b3e52642a92 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-agent/package.json +++ b/dev-packages/e2e-tests/test-applications/cloudflare-agent/package.json @@ -17,7 +17,7 @@ "@cloudflare/ai-chat": "^0.10.0", "@sentry/cloudflare": "^10.68.0", "@sentry/core": "^10.68.0", - "agents": "^0.20.0", + "agents": "latest", "ai": "^6.0.235", "react": "^19.2.8", "react-dom": "^19.2.8", @@ -41,6 +41,9 @@ "wrangler": "^4.114.0", "ws": "^8.21.1" }, + "sentryTest": { + "optional": true + }, "volta": { "node": "24.15.0", "extends": "../../package.json" diff --git a/packages/cloudflare/src/durableobject.ts b/packages/cloudflare/src/durableobject.ts index 3f3e96829df7..6077b1881235 100644 --- a/packages/cloudflare/src/durableobject.ts +++ b/packages/cloudflare/src/durableobject.ts @@ -1,12 +1,12 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { captureException, isObjectLike } from '@sentry/core'; +import { isObjectLike } from '@sentry/core'; import type { DurableObject } from 'cloudflare:workers'; import { setAsyncLocalStorageAsyncContextStrategy } from './async'; import type { CloudflareOptions } from './client'; -import { ensureInstrumented, getInstrumented, markAsInstrumented } from './instrument'; +import { getInstrumented, markAsInstrumented } from './instrument'; +import { instrumentDurableObjectHandlers } from './instrumentations/instrumentDurableObjectHandlers'; import { instrumentEnv } from './instrumentations/worker/instrumentEnv'; import { getFinalOptions } from './options'; -import { wrapRequestHandler } from './request'; import { instrumentContext } from './utils/instrumentContext'; import { hasRpcMeta } from './utils/rpcMeta'; import { getEffectiveRpcPropagation } from './utils/rpcOptions'; @@ -139,84 +139,6 @@ function resolveFrameworkManagedMethods( return managed; } -/** - * Instruments the built-in Durable Object handler methods on a constructed instance. - * - * These are the methods that are available on a Durable Object - * ref: https://developers.cloudflare.com/durable-objects/api/base/ - * - obj.alarm - * - obj.fetch - * - obj.webSocketError - * - obj.webSocketClose - * - obj.webSocketMessage - * - * Any other public methods on the Durable Object instance are RPC calls. - */ -function instrumentDurableObjectHandlers>( - obj: T, - options: CloudflareOptions, - context: InstrumentedDurableObjectContext, -): void { - // Bind each built-in handler to this instance before wrapping. - // See https://github.com/getsentry/sentry-javascript/issues/22328 - if (obj.fetch && typeof obj.fetch === 'function') { - obj.fetch = ensureInstrumented( - obj.fetch.bind(obj), - original => - new Proxy(original, { - apply(target, thisArg, args) { - return wrapRequestHandler({ options, request: args[0], context }, () => { - return Reflect.apply(target, thisArg, args); - }); - }, - }), - ); - } - - if (obj.alarm && typeof obj.alarm === 'function') { - // Alarms are independent invocations, so we start a new trace and link to the previous alarm - obj.alarm = wrapMethodWithSentry( - { - options, - context, - spanName: 'alarm', - spanOp: 'function', - startNewTrace: true, - origin: 'auto.faas.cloudflare.durable_object', - }, - obj.alarm.bind(obj), - ); - } - - if (obj.webSocketMessage && typeof obj.webSocketMessage === 'function') { - obj.webSocketMessage = wrapMethodWithSentry( - { options, context, spanName: 'webSocketMessage', origin: 'auto.faas.cloudflare.durable_object' }, - obj.webSocketMessage.bind(obj), - ); - } - - if (obj.webSocketClose && typeof obj.webSocketClose === 'function') { - obj.webSocketClose = wrapMethodWithSentry( - { options, context, spanName: 'webSocketClose', origin: 'auto.faas.cloudflare.durable_object' }, - obj.webSocketClose.bind(obj), - ); - } - - if (obj.webSocketError && typeof obj.webSocketError === 'function') { - obj.webSocketError = wrapMethodWithSentry( - { options, context, spanName: 'webSocketError', origin: 'auto.faas.cloudflare.durable_object' }, - obj.webSocketError.bind(obj), - (_, error) => - captureException(error, { - mechanism: { - type: 'auto.faas.cloudflare.durable_object_websocket', - handled: false, - }, - }), - ); - } -} - type RpcInstanceState = { options: CloudflareOptions; context: InstrumentedDurableObjectContext; diff --git a/packages/cloudflare/src/instrumentations/instrumentDurableObjectHandlers.ts b/packages/cloudflare/src/instrumentations/instrumentDurableObjectHandlers.ts new file mode 100644 index 000000000000..3cf434559fda --- /dev/null +++ b/packages/cloudflare/src/instrumentations/instrumentDurableObjectHandlers.ts @@ -0,0 +1,161 @@ +/* eslint-disable @typescript-eslint/unbound-method */ +import { captureException, debug } from '@sentry/core'; +import type { DurableObject } from 'cloudflare:workers'; +import type { CloudflareOptions } from '../client'; +import { DEBUG_BUILD } from '../debug-build'; +import { ensureInstrumented } from '../instrument'; +import { wrapRequestHandler } from '../request'; +import { wrapMethodWithSentry } from '../wrapMethodWithSentry'; + +/** + * The instrumented context of the Durable Object being wrapped. + * + * Kept as `any` for the same reason as in `durableobject.ts`: a concrete `DurableObjectState` here + * makes `tsc` relate its `SqlStorage` graph against the parameter union of `wrapMethodWithSentry`, + * which hangs the type build. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type InstrumentedDurableObjectContext = any; + +/** + * Instruments the built-in Durable Object handler methods on a constructed instance. + * + * These are the methods that are available on a Durable Object + * ref: https://developers.cloudflare.com/durable-objects/api/base/ + * - obj.alarm + * - obj.fetch + * - obj.webSocketError + * - obj.webSocketClose + * - obj.webSocketMessage + * + * Any other public methods on the Durable Object instance are RPC calls. + * + * @internal + */ +export function instrumentDurableObjectHandlers>( + obj: T, + options: CloudflareOptions, + context: InstrumentedDurableObjectContext, +): void { + // Bind each built-in handler to this instance before wrapping. + // See https://github.com/getsentry/sentry-javascript/issues/22328 + if (obj.fetch && typeof obj.fetch === 'function') { + setInstanceHandler( + obj, + 'fetch', + ensureInstrumented( + obj.fetch.bind(obj), + original => + new Proxy(original, { + apply(target, thisArg, args) { + return wrapRequestHandler({ options, request: args[0], context }, () => { + return Reflect.apply(target, thisArg, args); + }); + }, + }), + ), + ); + } + + if (obj.alarm && typeof obj.alarm === 'function') { + // Alarms are independent invocations, so we start a new trace and link to the previous alarm + setInstanceHandler( + obj, + 'alarm', + wrapMethodWithSentry( + { + options, + context, + spanName: 'alarm', + spanOp: 'function', + startNewTrace: true, + origin: 'auto.faas.cloudflare.durable_object', + }, + obj.alarm.bind(obj), + ), + ); + } + + if (obj.webSocketMessage && typeof obj.webSocketMessage === 'function') { + setInstanceHandler( + obj, + 'webSocketMessage', + wrapMethodWithSentry( + { + options, + context, + spanName: 'webSocketMessage', + origin: 'auto.faas.cloudflare.durable_object', + }, + obj.webSocketMessage.bind(obj), + ), + ); + } + + if (obj.webSocketClose && typeof obj.webSocketClose === 'function') { + setInstanceHandler( + obj, + 'webSocketClose', + wrapMethodWithSentry( + { + options, + context, + spanName: 'webSocketClose', + origin: 'auto.faas.cloudflare.durable_object', + }, + obj.webSocketClose.bind(obj), + ), + ); + } + + if (obj.webSocketError && typeof obj.webSocketError === 'function') { + setInstanceHandler( + obj, + 'webSocketError', + wrapMethodWithSentry( + { + options, + context, + spanName: 'webSocketError', + origin: 'auto.faas.cloudflare.durable_object', + }, + obj.webSocketError.bind(obj), + (_, error) => + captureException(error, { + mechanism: { + type: 'auto.faas.cloudflare.durable_object_websocket', + handled: false, + }, + }), + ), + ); + } +} + +/** + * Installs an instrumented handler as an own property of the Durable Object instance. + * + * A plain assignment is not always possible. The `agents` package installs its handlers with + * `Object.defineProperty(instance, name, { value, configurable: true })`, and `defineProperty` + * leaves `writable` at `false`. Assigning to such a property throws a `TypeError` in strict mode, + * so a read-only property is redefined instead. When the property can be neither assigned nor + * redefined, the handler stays uninstrumented rather than breaking the object. + */ +function setInstanceHandler(obj: object, name: string, handler: unknown): void { + const descriptor = Object.getOwnPropertyDescriptor(obj, name); + + try { + if (descriptor?.writable === false) { + Object.defineProperty(obj, name, { + value: handler, + writable: true, + enumerable: descriptor.enumerable, + configurable: descriptor.configurable, + }); + } else { + (obj as Record)[name] = handler; + } + } catch (error) { + DEBUG_BUILD && debug.warn(`Failed to instrument Durable Object handler "${name}"`, error); + } +} diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index 3039a06daf12..111d12fa1054 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -408,6 +408,57 @@ describe('instrumentDurableObjectWithSentry', () => { expect(obj.rpcMethod()).toBe('rpc-result'); }); + it('instruments built-in handlers installed as read-only own properties', () => { + // Shape installed by `agents` >= 0.22: `defineProperty` without `writable`, so the handlers + // are read-only and a plain assignment would throw in strict mode. + const testClass = class { + constructor() { + for (const name of ['fetch', 'alarm', 'webSocketMessage', 'webSocketClose', 'webSocketError']) { + Object.defineProperty(this, name, { + value: () => name, + configurable: true, + }); + } + } + }; + + const instrumented = instrumentDurableObjectWithSentry(vi.fn().mockReturnValue({}), testClass as any); + + let obj: any; + expect(() => { + obj = Reflect.construct(instrumented, [{ waitUntil: vi.fn() }, {}]); + }).not.toThrow(); + + for (const name of ['fetch', 'alarm', 'webSocketMessage', 'webSocketClose', 'webSocketError']) { + expect(getInstrumented(obj[name]), `Handler ${name} is instrumented`).toBeTruthy(); + } + + expect(obj.webSocketMessage()).toBe('webSocketMessage'); + }); + + it('leaves sealed own-property handlers untouched instead of failing construction', () => { + const originalHandler = (): string => 'sealed-result'; + const testClass = class { + constructor() { + Object.defineProperty(this, 'webSocketMessage', { + value: originalHandler, + writable: false, + configurable: false, + }); + } + }; + + const instrumented = instrumentDurableObjectWithSentry(vi.fn().mockReturnValue({}), testClass as any); + + let obj: any; + expect(() => { + obj = Reflect.construct(instrumented, [{ waitUntil: vi.fn() }, {}]); + }).not.toThrow(); + + expect(obj.webSocketMessage).toBe(originalHandler); + expect(obj.webSocketMessage()).toBe('sealed-result'); + }); + it('does not wrap Object.prototype methods as RPC methods', () => { const testClass = class { rpcMethod() {