Skip to content

Commit f820401

Browse files
dmmulroynicohrubec
andauthored
fix(cloudflare): use correct Proxy receiver in instrumentDurableObjectStorage (#19662)
## Summary Fixes #19661 `instrumentDurableObjectStorage`'s Proxy `get` trap passes `receiver` (the proxy) to `Reflect.get`, breaking native workerd getters like `storage.sql` that validate `this` via internal slots. - Change `Reflect.get(target, prop, receiver)` → `Reflect.get(target, prop, target)` so native getters execute with the real storage object as `this` - Add regression tests using a class with private fields to simulate workerd's native brand-checked getters ## Details The `sql` property on `DurableObjectStorage` is a native getter that requires the real native object as `this`. When the Proxy's `get` trap calls `Reflect.get(target, prop, receiver)`, the getter runs with `this` = proxy → "Illegal invocation". Using `target` as receiver ensures native getters always run against the real storage object. Instrumented KV methods (`get`, `put`, `delete`, `list`) were unaffected because they're functions that get explicitly `.bind(target)`ed or called via `.apply(target, args)`. The bug only manifests for non-function getters (like `sql`). **Regression tests** use a `BrandCheckedStorage` class with private fields — accessing `#sqlInstance` on the wrong `this` throws TypeError, faithfully simulating workerd's native internal-slot validation. --------- Co-authored-by: Nicolas Hrubec <nico.hrubec@sentry.io>
1 parent 0ff0468 commit f820401

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

packages/cloudflare/src/instrumentations/instrumentDurableObjectStorage.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,13 @@ type StorageMethod = (typeof STORAGE_METHODS_TO_INSTRUMENT)[number];
1616
*/
1717
export function instrumentDurableObjectStorage(storage: DurableObjectStorage): DurableObjectStorage {
1818
return new Proxy(storage, {
19-
get(target, prop, receiver) {
20-
const original = Reflect.get(target, prop, receiver);
19+
get(target, prop, _receiver) {
20+
// Use `target` as the receiver instead of the proxy (`_receiver`).
21+
// Native workerd getters (e.g., `storage.sql`) validate `this` via
22+
// internal slots. Passing the proxy as receiver breaks that check,
23+
// causing "Illegal invocation: function called with incorrect `this`
24+
// reference" errors.
25+
const original = Reflect.get(target, prop, target);
2126

2227
if (typeof original !== 'function') {
2328
return original;

packages/cloudflare/test/instrumentDurableObjectStorage.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,25 @@ describe('instrumentDurableObjectStorage', () => {
182182
});
183183
});
184184

185+
describe('native getter preservation', () => {
186+
it('preserves native getter `this` binding through the proxy', () => {
187+
// Private fields simulate workerd's native brand check —
188+
// accessing #sqlInstance on wrong `this` throws TypeError,
189+
// like workerd's "Illegal invocation".
190+
class BrandCheckedStorage {
191+
#sqlInstance = { exec: () => {} };
192+
get sql() {
193+
return this.#sqlInstance;
194+
}
195+
}
196+
197+
const storage = new BrandCheckedStorage();
198+
const instrumented = instrumentDurableObjectStorage(storage as any);
199+
200+
expect(() => (instrumented as any).sql).not.toThrow();
201+
});
202+
});
203+
185204
describe('error handling', () => {
186205
it('propagates errors from storage operations', async () => {
187206
const mockStorage = createMockStorage();

0 commit comments

Comments
 (0)