Skip to content

Commit

Permalink
fix: allow getPlatformProxy to handle RPC calls returning objects (#…
Browse files Browse the repository at this point in the history
…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 <5822355+RamIdeas@users.noreply.github.com>
  • Loading branch information
dario-piotrowicz and RamIdeas authored Jul 30, 2024
1 parent 7588800 commit 44ad2c7
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 3 deletions.
9 changes: 9 additions & 0 deletions .changeset/polite-dodos-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"miniflare": patch
---

fix: Allow the magic proxy to proxy objects containing functions indexed by symbols

In https://github.com/cloudflare/workers-sdk/pull/5670 we introduced the possibility
of the magic proxy to handle object containing functions, the implementation didn't
account for functions being indexed by symbols, address such issue
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,15 @@ describe("getPlatformProxy - env", () => {
rpc = env.MY_RPC as unknown as EntrypointService;
return dispose;
});
it("can call RPC methods directly", async () => {
it("can call RPC methods returning a string", async () => {
expect(await rpc.sum([1, 2, 3])).toMatchInlineSnapshot(`6`);
});
it("can call RPC methods returning an object", async () => {
expect(await rpc.sumObj([1, 2, 3, 5])).toEqual({
isObject: true,
value: 11,
});
});
it("can call RPC methods returning a Response", async () => {
const resp = await rpc.asJsonResponse([1, 2, 3]);
expect(resp.status).toMatchInlineSnapshot(`200`);
Expand Down
8 changes: 8 additions & 0 deletions fixtures/get-platform-proxy/workers/rpc-worker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ export class NamedEntrypoint extends WorkerEntrypoint {
sum(args: number[]): number {
return args.reduce((a, b) => a + b);
}

sumObj(args: number[]): { isObject: true; value: number } {
return {
isObject: true,
value: args.reduce((a, b) => a + b),
};
}

asJsonResponse(args: unknown): {
status: number;
text: () => Promise<string>;
Expand Down
11 changes: 9 additions & 2 deletions packages/miniflare/src/workers/core/proxy.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,15 @@ function isPlainObject(value: unknown) {
Object.getOwnPropertyNames(proto).sort().join("\0") === objectProtoNames
);
}
function objectContainsFunctions(obj: Record<string, unknown>): boolean {
for (const [, entry] of Object.entries(obj)) {
function objectContainsFunctions(
obj: Record<string | symbol, unknown>
): boolean {
const propertyNames = Object.getOwnPropertyNames(obj);
const propertySymbols = Object.getOwnPropertySymbols(obj);
const properties = [...propertyNames, ...propertySymbols];

for (const property of properties) {
const entry = obj[property];
if (typeof entry === "function") {
return true;
}
Expand Down
98 changes: 98 additions & 0 deletions packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,104 @@ test("Miniflare: service binding to named entrypoint", async (t) => {
});
});

test("Miniflare: service binding to named entrypoint that implements a method returning a plain object", async (t) => {
const mf = new Miniflare({
workers: [
{
name: "a",
serviceBindings: {
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
},
compatibilityFlags: ["rpc"],
modules: true,
script: `
export default {
async fetch(request, env) {
const obj = await env.RPC_SERVICE.getObject();
return Response.json({ obj });
}
}
`,
},
{
name: "b",
modules: true,
script: `
import { WorkerEntrypoint } from "cloudflare:workers";
export class RpcEntrypoint extends WorkerEntrypoint {
getObject() {
return {
isPlainObject: true,
value: 123,
}
}
}
`,
},
],
});
t.teardown(() => mf.dispose());

const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
const o = await bindings.RPC_SERVICE.getObject();
t.deepEqual(o.isPlainObject, true);
t.deepEqual(o.value, 123);
});

test("Miniflare: service binding to named entrypoint that implements a method returning an RpcTarget instance", async (t) => {
const mf = new Miniflare({
workers: [
{
name: "a",
serviceBindings: {
RPC_SERVICE: { name: "b", entrypoint: "RpcEntrypoint" },
},
compatibilityFlags: ["rpc"],
modules: true,
script: `
export default {
async fetch(request, env) {
const rpcTarget = await env.RPC_SERVICE.getRpcTarget();
return Response.json(rpcTarget.id);
}
}
`,
},
{
name: "b",
modules: true,
script: `
import { WorkerEntrypoint, RpcTarget } from "cloudflare:workers";
export class RpcEntrypoint extends WorkerEntrypoint {
getRpcTarget() {
return new SubService("test-id");
}
}
class SubService extends RpcTarget {
#id
constructor(id) {
super()
this.#id = id
}
get id() {
return this.#id
}
}
`,
},
],
});
t.teardown(() => mf.dispose());

const bindings = await mf.getBindings<{ RPC_SERVICE: any }>();
const rpcTarget = await bindings.RPC_SERVICE.getRpcTarget();
t.deepEqual(rpcTarget.id, "test-id");
});

test("Miniflare: custom outbound service", async (t) => {
const mf = new Miniflare({
workers: [
Expand Down

0 comments on commit 44ad2c7

Please sign in to comment.