Allow channel tokens to be created asynchronously#6724
Conversation
This mostly reverts commit 0d86b66. This removes the new `debugContext` string that was being passed around to distinguish params from results. Now that we've debugged the issue, this is more noise than it is worth. We do keep the `cap.debugInfo()` debug log on failures, since that's not so invasive and is more useful anyway.
DO NOT MERGE until the autogate has been rolled to all of production!
This allows ExternalPusher methods to continue to be invoked after the top-level RPC call(). (DO NOT MERGE until jsrpc-session-handle autogate is rolled out in prod.)
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR enables asynchronous channel token creation for SubrequestChannel/ActorClassChannel (using ExternalPusher + DelayedChannelToken), removes the now-fully-rolled-out StreamSink and ServerTopLevelMembrane code paths, and cleans up the corresponding autogates.
Findings (sorted by severity):
- [LOW] Comment for
getActorClass()incorrectly says it dispatches togetSubrequestChannelResolved()— should begetActorClassResolved(). - [LOW] Typo
prmoisein http.c++ and actor.c++ (introduced in this PR). - [LOW] Typos in comments: "Undelying" and "implemnetation" (two locations).
This review was generated by an AI assistant. Findings may contain false positives; please use your judgement.
|
Review posted successfully on PR #6724. Here's a summary of my findings: Overall assessment: This is a well-structured PR. The architecture for async channel tokens using Findings posted (all low severity):
All findings include concrete |
kentonv
force-pushed
the
kenton/async-channel-tokens
branch
from
3ff9ba6 to
7160d57
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6724 +/- ##
==========================================
- Coverage 66.56% 66.40% -0.17%
==========================================
Files 402 402
Lines 115885 116078 +193
Branches 19410 19437 +27
==========================================
- Hits 77142 77077 -65
- Misses 27162 27386 +224
- Partials 11581 11615 +34 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kentonv
force-pushed
the
kenton/async-channel-tokens
branch
from
7160d57 to
2cddddb
Compare
There are cases where it is difficult to acquire the channel token for a SubrequestChannel or ActorClassChannel synchronously, but until now we have needed to do so in order to serialize `Fetcher`s and `DurableObjectClass`es. We can't make serialization itself be async, because this would mess up e-order: A call that needs to wait for something while serializing params might end up being delayed until after some subsequent call which didn't wait, and so would be delivered out-of-order. To avoid this, we make it possible for a call to be sent with an IOU for the channel tokens. This uses `ExternalPusher`. The call embeds an external which is a promise capability. Later, the caller invokes the callee's `ExternalPusher` to push the channel token to it, and resolves the IOU promise to the resulting object. The callee can then unwrap the promise to get their token. (Opus 4.7 wrote the new test cases in channel-token-test but the rest of the code was by hand.)
This makes it so `getSubrequestChannel()` and similar methods of `IoChannelFactory` make sure that the contents of a `props` cap table are fully resolved before forwarding on to the `IoChannelFactory` implementation. This means that the underlying implementation of `getSubrequestChannelResolved()` et al doens't need to change to start calling `getResolved()` before trying to downcast channel objects to implementation-specific subclasses. This otherwise would have been really annoying to do in the internal codebase. Relatedly, this adds an `ensureAllResolved()` method to `DynamicWorkerSource`, for resolving channels there.
kentonv
force-pushed
the
kenton/async-channel-tokens
branch
from
2cddddb to
4ebf9af
Compare
2 participants
There are cases where it is difficult to acquire the channel token for a SubrequestChannel or ActorClassChannel synchronously, but until now we have needed to do so in order to serialize
Fetchers andDurableObjectClasses.We can't make serialization itself be async, because this would mess up e-order: A call that needs to wait for something while serializing params might end up being delayed until after some subsequent call which didn't wait, and so would be delivered out-of-order.
To avoid this, we make it possible for a call to be sent with an IOU for the channel tokens. This uses
ExternalPusher. The call embeds an external which is a promise capability. Later, the caller invokes the callee'sExternalPusherto push the channel token to it, and resolves the IOU promise to the resulting object. The callee can then unwrap the promise to get their token.This PR also, in preparation for landing the above, does some cleanup:
These two cleanups require that the respective autogates have rolled out, namely
rpc-use-external-pusherandjsrpc-session-handle. DO NOT MERGE this PR until those have rolled out! I am leaving this marked as "draft" until that time.