Skip to content

Commit 44ad2c7

Browse files
fix: allow getPlatformProxy to handle RPC calls returning objects (#6301)
* fix: allow `getPlatformProxy` to handle RPC calls returning objects allow the `getPlatformProxy` to handle RPC calls returning objects by making sure that the miniflare magic proxy can take into account methods set via symbols (in the case or RPC objects specifically `Symbol.dispose` and `Symbol.asyncDispose`) additionally add more RPC test coverage in miniflare --------- Co-authored-by: Rahul Sethi <[email protected]>
1 parent 7588800 commit 44ad2c7

File tree

5 files changed

+131
-3
lines changed

5 files changed

+131
-3
lines changed

.changeset/polite-dodos-happen.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"miniflare": patch
3+
---
4+
5+
fix: Allow the magic proxy to proxy objects containing functions indexed by symbols
6+
7+
In https://github.com/cloudflare/workers-sdk/pull/5670 we introduced the possibility
8+
of the magic proxy to handle object containing functions, the implementation didn't
9+
account for functions being indexed by symbols, address such issue

fixtures/get-platform-proxy/tests/get-platform-proxy.env.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,15 @@ describe("getPlatformProxy - env", () => {
188188
rpc = env.MY_RPC as unknown as EntrypointService;
189189
return dispose;
190190
});
191-
it("can call RPC methods directly", async () => {
191+
it("can call RPC methods returning a string", async () => {
192192
expect(await rpc.sum([1, 2, 3])).toMatchInlineSnapshot(`6`);
193193
});
194+
it("can call RPC methods returning an object", async () => {
195+
expect(await rpc.sumObj([1, 2, 3, 5])).toEqual({
196+
isObject: true,
197+
value: 11,
198+
});
199+
});
194200
it("can call RPC methods returning a Response", async () => {
195201
const resp = await rpc.asJsonResponse([1, 2, 3]);
196202
expect(resp.status).toMatchInlineSnapshot(`200`);

fixtures/get-platform-proxy/workers/rpc-worker/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ export class NamedEntrypoint extends WorkerEntrypoint {
1212
sum(args: number[]): number {
1313
return args.reduce((a, b) => a + b);
1414
}
15+
16+
sumObj(args: number[]): { isObject: true; value: number } {
17+
return {
18+
isObject: true,
19+
value: args.reduce((a, b) => a + b),
20+
};
21+
}
22+
1523
asJsonResponse(args: unknown): {
1624
status: number;
1725
text: () => Promise<string>;

packages/miniflare/src/workers/core/proxy.worker.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,15 @@ function isPlainObject(value: unknown) {
6868
Object.getOwnPropertyNames(proto).sort().join("\0") === objectProtoNames
6969
);
7070
}
71-
function objectContainsFunctions(obj: Record<string, unknown>): boolean {
72-
for (const [, entry] of Object.entries(obj)) {
71+
function objectContainsFunctions(
72+
obj: Record<string | symbol, unknown>
73+
): boolean {
74+
const propertyNames = Object.getOwnPropertyNames(obj);
75+
const propertySymbols = Object.getOwnPropertySymbols(obj);
76+
const properties = [...propertyNames, ...propertySymbols];
77+
78+
for (const property of properties) {
79+
const entry = obj[property];
7380
if (typeof entry === "function") {
7481
return true;
7582
}

packages/miniflare/test/index.spec.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,104 @@ test("Miniflare: service binding to named entrypoint", async (t) => {
821821
});
822822
});
823823

824+
test("Miniflare: service binding to named entrypoint that implements a method returning a plain object", async (t) => {
825+
const mf = new Miniflare({
826+
workers: [
827+
{
828+
name: "a",
829+
serviceBindings: {
830+
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
831+
},
832+
compatibilityFlags: ["rpc"],
833+
modules: true,
834+
script: `
835+
export default {
836+
async fetch(request, env) {
837+
const obj = await env.RPC_SERVICE.getObject();
838+
return Response.json({ obj });
839+
}
840+
}
841+
`,
842+
},
843+
{
844+
name: "b",
845+
modules: true,
846+
script: `
847+
import { WorkerEntrypoint } from "cloudflare:workers";
848+
export class RpcEntrypoint extends WorkerEntrypoint {
849+
getObject() {
850+
return {
851+
isPlainObject: true,
852+
value: 123,
853+
}
854+
}
855+
}
856+
`,
857+
},
858+
],
859+
});
860+
t.teardown(() => mf.dispose());
861+
862+
const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
863+
const o = await bindings.RPC_SERVICE.getObject();
864+
t.deepEqual(o.isPlainObject, true);
865+
t.deepEqual(o.value, 123);
866+
});
867+
868+
test("Miniflare: service binding to named entrypoint that implements a method returning an RpcTarget instance", async (t) => {
869+
const mf = new Miniflare({
870+
workers: [
871+
{
872+
name: "a",
873+
serviceBindings: {
874+
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
875+
},
876+
compatibilityFlags: ["rpc"],
877+
modules: true,
878+
script: `
879+
export default {
880+
async fetch(request, env) {
881+
const rpcTarget = await env.RPC_SERVICE.getRpcTarget();
882+
return Response.json(rpcTarget.id);
883+
}
884+
}
885+
`,
886+
},
887+
{
888+
name: "b",
889+
modules: true,
890+
script: `
891+
import { WorkerEntrypoint, RpcTarget } from "cloudflare:workers";
892+
893+
export class RpcEntrypoint extends WorkerEntrypoint {
894+
getRpcTarget() {
895+
return new SubService("test-id");
896+
}
897+
}
898+
899+
class SubService extends RpcTarget {
900+
#id
901+
902+
constructor(id) {
903+
super()
904+
this.#id = id
905+
}
906+
907+
get id() {
908+
return this.#id
909+
}
910+
}
911+
`,
912+
},
913+
],
914+
});
915+
t.teardown(() => mf.dispose());
916+
917+
const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
918+
const rpcTarget = await bindings.RPC_SERVICE.getRpcTarget();
919+
t.deepEqual(rpcTarget.id, "test-id");
920+
});
921+
824922
test("Miniflare: custom outbound service", async (t) => {
825923
const mf = new Miniflare({
826924
workers: [

0 commit comments

Comments
 (0)