Skip to content

Commit 5e71dc4

Browse files
committed
fix(cloudflare): use correct Proxy receiver in instrumentDurableObjectStorage
The storage Proxy's get trap passed `receiver` (the proxy itself) to `Reflect.get`, causing native workerd getters like `storage.sql` to throw 'Illegal invocation' due to failed internal slot / brand checks. Use `target` as the receiver so native getters execute with the correct `this` binding.
1 parent c3fa288 commit 5e71dc4

File tree

2 files changed

+89
-2
lines changed

2 files changed

+89
-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: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,41 @@ describe('instrumentDurableObjectStorage', () => {
182182
});
183183
});
184184

185+
describe('native getter preservation (sql)', () => {
186+
it('preserves native getter `this` binding for sql accessor', () => {
187+
// Simulates workerd's native DurableObjectStorage where `sql` is a
188+
// getter that validates `this` via internal slots (brand check).
189+
// Using a private field as the closest JS equivalent of a native
190+
// internal slot check — accessing `#sqlInstance` on the wrong `this`
191+
// throws a TypeError, just like workerd's "Illegal invocation".
192+
const storage = createBrandCheckedStorage();
193+
const instrumented = instrumentDurableObjectStorage(storage as any);
194+
195+
// Before fix: this threw "Cannot read private member #sqlInstance
196+
// from an object whose class did not declare it" (equivalent of
197+
// workerd's "Illegal invocation: function called with incorrect
198+
// `this` reference")
199+
expect(() => (instrumented as any).sql).not.toThrow();
200+
expect((instrumented as any).sql).toBeDefined();
201+
expect((instrumented as any).sql.exec).toBeDefined();
202+
});
203+
204+
it('sql.exec works through the instrumented proxy', () => {
205+
const storage = createBrandCheckedStorage();
206+
const instrumented = instrumentDurableObjectStorage(storage as any);
207+
208+
const result = (instrumented as any).sql.exec('SELECT 1');
209+
expect(result).toEqual({ query: 'SELECT 1' });
210+
});
211+
212+
it('non-instrumented methods preserve native `this` binding', () => {
213+
const storage = createBrandCheckedStorage();
214+
const instrumented = instrumentDurableObjectStorage(storage as any);
215+
216+
expect(() => (instrumented as any).getAlarm()).not.toThrow();
217+
});
218+
});
219+
185220
describe('error handling', () => {
186221
it('propagates errors from storage operations', async () => {
187222
const mockStorage = createMockStorage();
@@ -210,3 +245,50 @@ function createMockStorage(): any {
210245
},
211246
};
212247
}
248+
249+
/**
250+
* Creates a storage mock that uses a class with private fields to simulate
251+
* workerd's native brand-checked getters. Private field access throws a
252+
* TypeError when `this` is not the original instance, mimicking the
253+
* "Illegal invocation" error from workerd native accessors.
254+
*/
255+
class BrandCheckedStorage {
256+
#sqlInstance = { exec: (query: string) => ({ query }) };
257+
#data = new Map<string, unknown>();
258+
259+
get sql() {
260+
// Accessing #sqlInstance implicitly checks that `this` is a real
261+
// BrandCheckedStorage instance. If `this` is a Proxy with wrong
262+
// receiver, this throws TypeError.
263+
return this.#sqlInstance;
264+
}
265+
266+
async get(key: string) {
267+
return this.#data.get(key);
268+
}
269+
async put(key: string, value: unknown) {
270+
this.#data.set(key, value);
271+
}
272+
async delete(key: string) {
273+
return this.#data.delete(key);
274+
}
275+
async list() {
276+
return new Map(this.#data);
277+
}
278+
async getAlarm() {
279+
return null;
280+
}
281+
async setAlarm(_scheduledTime: number) {}
282+
async deleteAlarm() {}
283+
async deleteAll() {
284+
this.#data.clear();
285+
}
286+
async sync() {}
287+
async transaction(cb: (txn: unknown) => unknown) {
288+
return cb(this);
289+
}
290+
}
291+
292+
function createBrandCheckedStorage() {
293+
return new BrandCheckedStorage();
294+
}

0 commit comments

Comments
 (0)