Repository navigation
Release pipeline on rejected rpc promises - #7637
Open
Caio-Nogueira wants to merge 1 commit into
Open
Caio-Nogueira wants to merge 1 commit into
Caio-Nogueira wants to merge 1 commit into
Conversation
Contributor
|
Since last review: 1 resolved, 0 still open, 0 new. Reviewed commit: 1efc2db2 · github run |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
jp4a50
reviewed
Oct 8, 2026
Contributor
|
LGTM modulo the two comments. |
Caio-Nogueira
force-pushed
the
caio/jsrpc-rejected-pipeline
branch
from
October 9, 2026 10:39
92de8a2 to
5e46cb4
Compare
jp4a50
approved these changes
Oct 9, 2026
jp4a50
enabled auto-merge
October 9, 2026 11:24
Caio-Nogueira
force-pushed
the
caio/jsrpc-rejected-pipeline
branch
from
October 9, 2026 14:15
5e46cb4 to
67b6b5e
Compare
Caio-Nogueira
force-pushed
the
caio/jsrpc-rejected-pipeline
branch
from
October 9, 2026 14:53
67b6b5e to
b95505e
Compare
A JsRpcPromise whose call rejects stays in its Pending state and keeps holding the call's pipeline until it is disposed or garbage collected. The pipeline goes through the callee session's CompletionMembrane, so holding it keeps the session open. A stateless callee's request then has no pending events but never completes, so the runtime aborts it as hung and its tail outcome becomes "exception". An actor callee's request never completes at all. js-rpc-rejected-pipeline-test holds rejected promises in four ways and asserts, through a tail worker, that the callee's invocation still completes with "ok": - a caller holding a rejected call into the callee, - a caller holding a rejected call into a Durable Object, - a callee holding a rejected call on a stub the caller passed in, - a caller holding a rejected call to a callback from the callee. workerd never reserves actor-call replay memory, so those calls do not use JsRpcCallRetryState. jsrpc-call-retry-test gains a case for that variant: a retryable actor call whose pipeline was used before it rejected, checking that the receiving actor's request finishes. All of these currently fail: the stateless cases with the hang abort, the others by timing out waiting for the callee's request to end. Release a JsRpcPromise's pipeline when its call rejects Only the success path moved JsRpcPromise out of its Pending state, so a rejected promise kept holding the call's pipeline until it was disposed or garbage collected. The header claimed this held nothing open on the server, but it does: the pipeline goes through the callee session's CompletionMembrane, keeping the membrane policy, and so the session, alive. The callee's request is then left with no pending events, and a stateless callee is aborted as hung once the caller's await settles. This also applies to a callee holding a rejected call on a stub its caller passed in, which pins the callee's own session. For example, Workflows sees user invocations aborted whenever run() throws or a step call rejects. Add a Rejected state that holds the call's error instead of the pipeline. A catch handler on the call's promise moves the JsRpcPromise there, covering both the plain and the retrying (JsRpcCallRetryState) call paths. Pipelining on a rejected promise still fails with the call's error. The catch handler converts the error to a kj::Exception once, before the application's handlers receive it, so later pipelined operations report the original error even if a handler mutates the JS object. Conversion can run application code, such as getters on Error.prototype, which can let GC destroy the promise, so the handler converts first and only then looks the promise up through its weak reference. If that code throws, the promise is still rejected, with a generic error for pipelined operations, and the caller still receives the call's original error. Rejected also keeps an IoPtr to the promise, as Resolved does, so pipelining on it from another request still fails with the cross-request error. js-rpc-test covers the cross-request case, and js-rpc-rejected-pipeline-test covers the mutation case and getters that collect the promise or throw during conversion.
Caio-Nogueira
force-pushed
the
caio/jsrpc-rejected-pipeline
branch
from
October 9, 2026 15:05
b95505e to
1efc2db
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
A
JsRpcPromisewhose call rejects stays in its Pending state and keeps holding the call's pipeline until it is disposed or garbage collected. Consequently the caller might see code hung events on its trace when it sees a rejected rpc promise on its lifetime.On workflows, with a bidirectional rpc flow, this happens on caller and callee (similar to the
calleeHoldsRejectedCallOnCallerStubtest case). I used a red-green approach that successfully reproduced the production issue.The fix introduced a
Rejectedstatus on rpc promises that carries the original error. When an rpc promise is rejected, we move it into the rejected state, release pipeline (which makes it no longer vulnerable to the incorrecterroredoutcome report), and track the error for gc