JSRPC: Support capnp-over-http for external server bindings #1757
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
jasnell
reviewed
Mar 3, 2024
| @@ -246,8 +246,8 @@ export let namedServiceBinding = { | |||
| assert.strictEqual(sawFinally, true); | |||
|
|
|||
| // `fetch()` is special, the params get converted into a Request. | |||
| let resp = await env.MyService.fetch("http://foo", {method: "POST"}); | |||
| assert.strictEqual(await resp.text(), "method = POST, url = http://foo"); | |||
| let resp = await env.MyService.fetch("http://foo/", {method: "POST"}); | |||
There was a problem hiding this comment.
Out of curiosity, why is adding the trailing / here necessary?
There was a problem hiding this comment.
When sending the request over actual HTTP, the trailing / gets added if it is missing. So in order to make the test consistent between when it runs over a real socket vs. local service binding, I had to add the /.
There was a problem hiding this comment.
hmm, ok. Per fetch spec the trailing slash is supposed to be implicit in this case so it technically should be fine to pass in without it (since the url arg here is technically supposed to be passed through the whatwg url parser if we're following spec)... so it just feels a bit odd to have to modify the test to make it work correctly. But I think this is fine for now. I do think at some point we need to revisit how the input url is handled on fetch calls tho.
There was a problem hiding this comment.
The problem is if I don't add the / here, then the assert on the next line fails either when using real HTTP (if it doesn't expect a /) or when not using real HTTP (if it does expect a /). In order to be able to write a single assert which works in both cases, I had to add the / on this line.
Arguably this is a bug in service bindings, that they do not implicitly add the / as regular HTTP would, but that bug has been around a while, not introduced in this PR.
jasnell
reviewed
Mar 3, 2024
jasnell
approved these changes
Mar 3, 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
mrbbot
approved these changes
Mar 4, 2024
Comment on lines
+30
to
+42
| ( name = "loop", | ||
| external = ( | ||
| address = "loopback:loop", | ||
| http = (capnpConnectHost = "cappy") | ||
| ) | ||
| ) | ||
| ], | ||
| sockets = [ | ||
| ( name = "loop", | ||
| address = "loopback:loop", | ||
| service = (name = "js-rpc-test", entrypoint = "MyService"), | ||
| http = (capnpConnectHost = "cappy") | ||
| ) |
There was a problem hiding this comment.
This should work well for us! Thanks! 😃
kentonv
force-pushed
the
kenton/jsrpc-over-http
branch
from
6719d3d
to
be71b89
Compare
Incoming listen sockets and outgoing ExternalService bindings can now both be configured to send/receive a special HTTP CONNECT request to open a Cap'n Proto connection. It is then possible to use JS RPC over such a binding. Thus, separate workerd intances can be configured to speak RPC to each other, even over a network.
A socket can be declared to listen on an address like `loopback:name`, and an ExternalService can connect to it. This namespace exists only in-process. This will allow us to test RPC while invoking the whole network stack.
Without creating a PendingEvent for every incoming RpcTarget, the IoContext may be canceled prematurely, because the system thinks that there's no way more events could arrive. This problem didn't come up when using direct service bindings because the PendingEvent system does not cancel the IoContext until the thread's event loop has been emptied, just before going back to the OS to wait for I/O. With local service bindings, the communications back and forth between the services all happens entirely in the local event loop without requiring any I/O, so the test completes before the PendingEvent system can kick in and cause a problem. When we use a real socket, communications between Workers requires actual OS I/O, so the PendingEvent has a chance to do its thing.
kentonv
force-pushed
the
kenton/jsrpc-over-http
branch
from
be71b89
to
bf992cf
Compare
|
Changed this to be based on #1756 so I don't have to rebase again later... |
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.
This PR makes it possible to configure incoming sockets and outgoing ExternalServers to allow negotiating a Cap'n Proto connection over HTTP. Miniflare will need this to allow RPC over service bindings to be tested, since miniflare currently implements service bindings by running each worker in a separate workerd instance.
Currently, this works by using an HTTP CONNECT request specifying a magic hostname (hostname chosen in the config). In theory it might be better to use
Upgrade, but that would require extensions to KJ HTTP. It could also make sense to use a WebSocket endpoint, but that would have been more work. We can support those as alternatives in the future if there is demand.I also went ahead and used this new code path to enhance js-rpc-test, so that it actually tests running over a real socket. This actually caught a bug!