JSRPC: Support passing RPC stubs across other RPCs by kentonv · Pull Request #1692 · cloudflare/workerd

kentonv · 2024-02-19T07:04:30Z

The first four commits (mainly the third commit) add a framework to allow JSG types to declare themselves serializable. @jasnell I know you were also working on something here so sorry if this overlaps. I did take some inspiration from your design doc. But, it occurred to me that instead of writing a tag and a version, we could combine these: whenever a type decides it needs a new serialization version, it allocates a whole new tag for it. This way we save some bytes on the wire.

The remaining commits, after some bug fixes and refactoring, introduce RpcTarget as a new type which classes can extend to mark themselves as being RPC objects -- without being a top-level entrypoint. Instances of these types can then be sent in RPC messages. The receiving side gets an RpcStub instead, which allows calls back to the original object.

Example:

import {WorkerEntrypoint,RpcTarget} from 'cloudflare:workers';

class Counter extends RpcTarget {
  i = 0;
  async increment(j) {
    this.i += j;
    return this.i;
  }
}

export class CounterService extends WorkerEntrypoint {
  async makeCounter() {
    return new Counter();
  }
}

let counter = await env.COUNTER_SERVICE.makeCounter();
await counter.increment(5);  // returns 5
await counter.increment(3);  // returns 8

NOT done in this PR, but needed before this is production-ready:

resource management
- close() methods
- auto-close parameter caps after call returns
- inject close method on returned object if it contains nested stubs
promise pipelining
streams, etc.

src/workerd/jsg/ser.c++

src/workerd/jsg/ser-test.c++

jasnell · 2024-02-20T21:28:07Z

src/workerd/jsg/ser.h

+//   is the format that `serialize()` will write. The other tags are non-current versions that
+//   `deserialize()` accepts in addition to the current version. These are usually old versions,
+//   but could also include a new version that hasn't fully rolled out yet -- it will be necessary
+//   to fully roll out support for parsing a new version before anyone can start generating it.


nit: worth mentioning the ONEWAY variant of the macro here?

src/workerd/jsg/ser.h

src/workerd/api/worker-rpc.c++

jasnell · 2024-02-20T21:47:30Z

Generally LGTM. Left some notes that can safely be handled later if you'd like.

geelen

LGTM. Only really reviewed the surface-level JS stuff & left a few comments, but nothing major.

src/cloudflare/internal/workers.d.ts

geelen · 2024-02-21T04:54:34Z

src/cloudflare/workers.ts

@@ -9,3 +9,5 @@ import entrypoints from 'cloudflare-internal:workers';



this could be just export * from 'cloudflare-internal:workers';

geelen · 2024-02-21T05:04:20Z

src/cloudflare/internal/workers.d.ts

+  public constructor(server: object);
+}
+
+export class RpcTarget {


should DurableObject (at least) and WorkerEntrypoint (maybe) inherit from RpcTarget as well?

For example, I'd like to return a DO stub from an RPC endpoint:

class MyObject extends DurableObject { async someMethod() { ... } } class MyService extends WorkerEntrypoint { static getInstance(accountId, sessionId) { const ns = this.ctx.self.MyObject const id = ns.idFromName(`${accountId}-${sessionId}`) return this.get(id) } }

And call it like this:

await env.MyService.getInstance(accountId, sessionId).someMethod()

or will that already work?

I thought about it a little bit but it's not clear to me. I think we may want to handle these in different ways. Consider that if you have a DO stub (you're a client of it) and you want to send that stub to someone else over a separate RPC, we probably want to implement that by sending the actor ID, not the live capability. The receiving worker can connect to that actor ID themselves. Similarly if you send a service binding then we can send the worker ID instead of a live capability.

But what happens if you are not a client, but you're doing return this inside the object implementation? If we're going to say these things behave like RpcTarget, then return this should return a capability to the specific instantiation? But it's weird that this is different from what happens if you send a copy of the binding / DO stub?

This needs more thought and I think we should punt for now.

See comments in `ser.h` for how it works.

…refs.

On further thought, this seems like a more accessible name for this type.

Instead of accepting / returning byte arrays, these now accept / build `rpc::JsValue`. This is important as `JsValue` will soon contain more than just serialized bytes.

Otherwise, all stubs are custom thenables! Yikes!

…tion. We need to store a weak ref. No biggie. Also, though, we need to make sure that once the call is running, it'll be canceled if the IoContext itself is canceled. So we need to addTask() it. This change also fixes a bug where if the client canceled the RPC and dropped the capability, the JS promise (which can't be canceled) would hold dangling pointers.

This factors out a JsRpcTargetBase base class, moving the logic specific to entrypoints into a new subclass EntrypointJsRpcTarget. In a future commit, another subclass will implement RPC delivery to non-entrypoint objects.

…Stubs. Objects which are meant to be called over RPC, but which are _not_ top-level entrypoints, must implement `RpcTarget`. An `RpcStub` can be constructed directly around an `RpcTarget`, like `new RpcStub(myTarget)`. This creates a loopback stub. This is mostly not useful in itself since the RPC system will do it automatically, but sometimes this is nice for testing, or when the same object is going to be sent multiple times and only one close() notification is desired (long story, and not implemenetd yet).

We define a new table of "externals" that rides along with the `rpc::JsValue` capnp struct, and use the new JS serialization features we created to make JsRpcStub serialize into it.

kentonv · 2024-02-21T23:57:01Z

Ugh botched a rebase again (my fixup script doesn't work correctly if main isn't synced, oops).

This is the change I actually made:

6cfa2fb

This restores the default error messages when serializing a non-serializable object. Otherwise some tests broke in the internal build.

- Consolidates Service binding documentation to be within the Runtime APIs section of the docs. Currently docs for configuring a Service binding, and docs for how to write code around Service bindings are in separate sections of the docs, which makes getting started hard, requires jumping back and forth between pages. Relevant content from [the configuration section](https://github.com/cloudflare/cloudflare-docs/blob/production/content/workers/configuration/bindings/about-service-bindings.md) has been moved here, and will be removed. - Explains what Service bindings are and what to use them for. - Provides separate code examples RPC and HTTP modes of working with Service bindings. refs cloudflare/workerd#1658, cloudflare/workerd#1692, cloudflare/workerd#1729, cloudflare/workerd#1756, cloudflare/workerd#1757

* [do not merge] Revamp Service Bindings docs for RPC - Consolidates Service binding documentation to be within the Runtime APIs section of the docs. Currently docs for configuring a Service binding, and docs for how to write code around Service bindings are in separate sections of the docs, which makes getting started hard, requires jumping back and forth between pages. Relevant content from [the configuration section](https://github.com/cloudflare/cloudflare-docs/blob/production/content/workers/configuration/bindings/about-service-bindings.md) has been moved here, and will be removed. - Explains what Service bindings are and what to use them for. - Provides separate code examples RPC and HTTP modes of working with Service bindings. refs cloudflare/workerd#1658, cloudflare/workerd#1692, cloudflare/workerd#1729, cloudflare/workerd#1756, cloudflare/workerd#1757 * Remove Service bindings config page, update links, redirects * Apply suggestions from code review * Further consolidate bindings content within Runtime APIs, link from config section * Redirect from config bindings to Runtime APIs bindings * Update links to point to Runtime APIs bindings section * Fix redirects * Fix linter warnings * Bold bullet points for Service Bindings explainer * Add missing bindings to /runtime-apis/bindings * Add env vars and secrets links to /runtime-apis/bindings/ section * Update content/workers/runtime-apis/bindings/ai.md * Update content/workers/runtime-apis/bindings/service-bindings.md * Apply suggestions from code review Co-authored-by: Matt Silverlock <msilverlock@cloudflare.com> * Break docs into RPC and HTTP sections * Moving over more docs * Fix titles * Fixes * More docs * More, need to break apart into pages * more * fixup * Apply suggestions from code review Co-authored-by: Michael Hart <michael.hart.au@gmail.com> Co-authored-by: Kenton Varda <kenton@cloudflare.com> * Remove unnecessary changes * Create RPC and Context sections * Rename to /visibility * Edits * Fix naming * Edits * Add note about Queues to context docs * Clarify language in RPC example * Clarify service binding performance * Link to fetch handler in describing HTTP service bindings * Move remaining content over from tour doc * Add limits section, note about Smart Placement * Edits * WorkerB => MyWorker * Edits plus partial * Update content/workers/runtime-apis/bindings/service-bindings/rpc.md * Edits * Making sure internal doc covered, minus Durable Objects docs * Remove extraneous section * Call out RPC lifecycle docs in Service Bindings section * Update content/workers/runtime-apis/rpc/lifecycle.md * Edits to JSRPC API docs (#13801) * Clarify structured clonability. - Let's not talk about class instances being an exception to structured clone early on. Instead, we can have an aside later down the page. Most people probably wouldn't even expect structured clone to treat classes this way anyway, so telling the about it just to tell them that we don't do that is distracting. - Adjust the wording in anticipation of the fact that we're likely to add many more types that can be serialized, and this list will likely not keep up. The important thing here is to explain the types that have special behavior (they aren't just data structures treated in the obivous way). - Briefly describe these special semantics in the list, to get people excited to read more. * Minor wording clarification. It was confusing whether "object" here meant a plain object or a class instance. * Clarify garbage collection discussion. The language here was not very precise, and would have confused people who have a deep understanding of garbage collectors. * Better link for V8 explicit resource management. The previous link pointed to a mailing list thread of messages generated by the bug tracker. Let's link directly to the bug tracker. * Fix typo. * Clarify language about disposers. The language here said that stubs "can be disposed" at the end of a using block, which implies that they might not be, or that this is some sort of hint to the GC. It's more accurate to say that they *will* be disposed, that is, their disposer *will* be called, completely independent of GC. The advice about when to use `using` was unclear. I've changed the advice to simply say that the return value of any call should be stored into `using`, which is an easy rule to follow. * Remove "Sessions" section from lifecycle. This section was placed under "automatic disposal" but doesn't seem to belong here. I don't think it's really necessary to define a "session" unless we are then going to say something about sessions, but the word doesn't appear anywhere else on the page. Sessions are closely related to execution contexts, but execution contexts were already described earlier. * Clarify section on automatic disposal. * Correct docs on promise pipelining. The previous language incorrectly suggested that promise pipelining would kick in even if the caller awaited each promise. In fact, it is necessary to remove the `await` in order to get the benefits. * Fix reserved methods doc. The doc incorrectly stated that `fetch` and `connect` were special on `RpcTarget` in addition to `WorkerEntrypoint` and `DurableObject`. This is not correct. The doc didn't cover the numerous other reserved method names. * elide -> omit Co-authored-by: Brendan Irvine-Broque <birvine-broque@cloudflare.com> --------- Co-authored-by: Brendan Irvine-Broque <birvine-broque@cloudflare.com> * Apply suggestions from code review Co-authored-by: Greg Brimble <gbrimble@cloudflare.com> Co-authored-by: Kenton Varda <kenton@cloudflare.com> * Apply suggestions from code review Co-authored-by: Greg Brimble <gbrimble@cloudflare.com> * More RPC doc tweaks: Move stuff around! (#13808) * Move section on proxying stubs to compatible-types. This isn't really lifecycle-related. But it is another kind of thing that can be sent over RPC. * Move "promise pipelining" to compatible-types under "class instances". Promise pipelining isn't really about lifecycle. I think it fits under "class instances" because it is motivated by class instances. * Merge compatible-types into RPC root doc. The compatible-types list ends up highlighting the key exciting features of the RPC system. This should be at the root. * Tweak RPC root page. I'm removing "How it Works" with the function example because: 1. The example itself doesn't really explain "how it works". 2. We now present this same example down the page under "Functions". * Add changelog entry * Update content/workers/runtime-apis/rpc/lifecycle.md Co-authored-by: Greg Brimble <gbrimble@cloudflare.com> * More more JSRPC doc tweaks (#13840) * Add documentation for `rpc` compat flag. * Update links to about-service-bindings. * Update content/workers/_partials/_platform-compatibility-dates/rpc.md * Update content/workers/runtime-apis/rpc/_index.md Co-authored-by: James M Snell <jasnell@gmail.com> * Update content/workers/_partials/_platform-compatibility-dates/rpc.md Co-authored-by: James M Snell <jasnell@gmail.com> * Named entrypoints (#13861) * Named entrypoint configuration in `wrangler.toml` * Named entrypoints example * Apply suggestions from code review * Apply suggestions from code review --------- Co-authored-by: Brendan Irvine-Broque <birvine-broque@cloudflare.com> * Apply suggestions from code review * Clarify RPC unsupported errors (#13863) * * Add Durable Objects RPC docs (#13765) * Update DO counter example with RPC * Clarify RPC pricing * Rename "Configuration" to "Best Practices" section * Fix some redirects (#13865) * Order the RPC docs sections in nav (#13866) * Fix links * Fix more redirects * Fix DO redirect in Versions & Deployments * fix merge conflict --------- Co-authored-by: Matt Silverlock <msilverlock@cloudflare.com> Co-authored-by: Michael Hart <michael.hart.au@gmail.com> Co-authored-by: Kenton Varda <kenton@cloudflare.com> Co-authored-by: Greg Brimble <gbrimble@cloudflare.com> Co-authored-by: James M Snell <jasnell@gmail.com> Co-authored-by: Vy Ton <vy@cloudflare.com> Co-authored-by: Rita Kozlov <rita@cloudflare.com> Co-authored-by: Rita Kozlov <2414910+rita3ko@users.noreply.github.com>

kentonv requested review from MellowYarker, geelen and jasnell February 19, 2024 07:04

kentonv requested review from a team as code owners February 19, 2024 07:04

kentonv requested a review from Warfields February 19, 2024 07:04