JSRPC: Promise Pipelining (and property access) #1729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kentonv
force-pushed
the
kenton/jsrpc-pipelining
branch
from
cc61e6d
to
d3a7914
Compare
jasnell
reviewed
Feb 25, 2024
jasnell
reviewed
Feb 25, 2024
jasnell
reviewed
Feb 25, 2024
jasnell
reviewed
Feb 25, 2024
jasnell
reviewed
Feb 25, 2024
jasnell
reviewed
Feb 25, 2024
kentonv
force-pushed
the
kenton/jsrpc-pipelining
branch
from
85c926e
to
14053ca
Compare
kentonv
force-pushed
the
kenton/jsrpc-pipelining
branch
from
14053ca
to
2bde662
Compare
|
Had to rebase over the JsMap change in order to resolve merge conflicts so that tests would run, but this means this branch is running ahead of what's merged downstream. |
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
jasnell
reviewed
Feb 27, 2024
| # Path of properties to follow from the JsRpcTarget itself to find the method being called. | ||
| # E.g. if the application does: | ||
| # | ||
| # myRpcTarget.foo.bar.baz() |
There was a problem hiding this comment.
Can/will we support myRpcTarget.foo.bar().baz.qux()?
There was a problem hiding this comment.
Yep, that works by extension.
|
Just a general comment here, how are we going to represent the return type of both an RpcTarget or a property in Typescript wrt promise pipelining? Both doing multiple awaits and relying on pipeline need to be valid: let value = await stub.someProperty
let x = value.inner
let y = await stub.someProperty.inner
await stub.someMethod().someOtherMethod();
await (await stub.someMethod()).someOtherMethod();@mrbbot @GregBrimble thoughts? |
kentonv
force-pushed
the
kenton/jsrpc-pipelining
branch
from
0aef184
to
ce1f3f8
Compare
MellowYarker
reviewed
Mar 1, 2024
…tion. This will allow us to support accessing properties that are _not_ actually methods in the comming commits.
There's no good reason for this to be StringPtr since the caller just allocated it, and if it's a String we can avoid some copies.
On the client side, you can specify a chain like:
stub.foo.bar.baz()
This sends a single call to the server, specifying the full path `foo.bar.baz` instead of a simple method name.
On the server side, only properties which are simple objects or derivatives of `RpcTarget` may be traversed. When traversing the property of a class (not a simple object), the property must be declared on the class, not on the instance.
You can just await a property.
let value = await stub.someProperty;
That's it. That's the feature.
Notes:
* The system only attempts to fetch the property if you await it. This is accomplished by implementing a custom thenable that only actually initiates the fetch when `.then()` is called.
* If the property's value is actually a Promise on the server side, it'll be awaited there before returning.
* As always, for classes, only properties declared on the class are allowed, not instance properties. (For regular objects, any property can be fetched.)
Calling an RPC now returns a special kind of promise which also has a wildcard property allowing you to speculatively access properties without awaiting.
Recent changes made it so that we send the CustomEvent to the WorkerInterface before the top-level RPC call has actually been constructed. But this means if we fail to serialize the parameters, the event is already running, and it will just hang open since the server side won't return until the client side drops all the capabilities. To fix this, we apply a membrane on the client side that cancels the requset when all capabilities have been dropped. This feels ugly. You might argue we should instead further complexify the signature of getClientForOneCall() so that it returns some sort of callback that can be invoked to say "ok, I successfully initiated a call", and then only send the CustomEvent when that's called. That would recreate the previous behavior. However, that approach is not fully correct either: If for some reason the call is sent but fails to make it all the way to the server, the same hang can happen. The new solution also has the advantage that if the last capability is dropped from the client side (which is probably the common case), then the event can be cleaned up immediately without waiting for a round trip to the server for the server to notice. There is a disadvantage, which is that most RPC sessions will appear to end in cancellation (e.g. in metrics). However, this is not that big a deal and we can find a fix for it later.
…ons. Since we're not relying on the usual liftKj() behavior to turn exceptions async, we should do it ourselves.
This isn't meant to work, but it does work in certain cases, and we need to avoid breaking those.
This was an oversight from earlier. I assumed these were already marked for cancellation. Unfortunately we are unable to mark the entire file worker-interface.capnp since runAlarm() actually depends on being not cancellable... something we need to fix. This actually fixes a test failure with process sandboxing.
kentonv
force-pushed
the
kenton/jsrpc-pipelining
branch
from
a6cf249
to
9babd0d
Compare
|
9babd0d is new, the rest of the above is a rebase. |
jasnell
approved these changes
Mar 1, 2024
MellowYarker
approved these changes
Mar 1, 2024
irvinebroque
added a commit
to cloudflare/cloudflare-docs
that referenced
this pull request
Mar 3, 2024
- 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
irvinebroque
added a commit
to cloudflare/cloudflare-docs
that referenced
this pull request
Mar 30, 2024
- 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
rita3ko
added a commit
to cloudflare/cloudflare-docs
that referenced
this pull request
Apr 5, 2024
* [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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
You can now access properties on an RPC object by just awaiting them:
And you can access properties on a promise returned by an RPC call, in order to make speculative calls on stubs that you expect to be returned:
Both of these are achieved via custom thenables rather than regular promises. This allows us to return a value that can be awaited, but which also has a wildcard property.