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
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
  • Loading branch information
dario-piotrowicz committed Jul 28, 2024
1 parent ff6b38b commit c65d040
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 2 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
10 changes: 9 additions & 1 deletion packages/miniflare/src/workers/core/proxy.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,15 @@ function isPlainObject(value: unknown) {
);
}
function objectContainsFunctions(obj: Record<string, unknown>): boolean {
for (const [, entry] of Object.entries(obj)) {
const propertyNames = Object.getOwnPropertyNames(obj);
const propertySymbols = Object.getOwnPropertySymbols(obj);
const properties = [...propertyNames, ...propertySymbols];

for (const property of properties) {
// @ts-ignore - ignoring the following line since TypeScript seems to
// incorrectly error if `property` is a symbol
// (see: https://github.com/Microsoft/TypeScript/issues/24587)
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 c65d040

Please sign in to comment.