Repository navigation
Port ExecutionContext to Rust behind the rust-execution-context autogate - #8256
bartlomieju wants to merge 11 commits into
Conversation
Under capture_async_api_throws, C++ JSG returns a rejected promise from a method, property or call handler whose result is a promise, instead of throwing (LiftKj_<v8::Local<v8::Promise>> in jsg/util.h). Argument conversion errors and errors from the method itself are both caught; a termination keeps propagating. Rust JSG threw them all synchronously, so a Rust port of such a method behaved differently from the C++ one it replaces, for example for `api.method(badArg).catch(...)`. The generated callback of a #[jsg_method], #[jsg_property] getter or #[jsg_callable] that returns a jsg::Promise or a v8::Local<v8::Promise>, alone or in a Result, now runs its body through jsg::capture_throws_as_rejection(). If the isolate captures throws as rejections, the body runs in Lock::try_catch(), and what it throws is returned as a rejected promise. Isolate::captures_throws_as_rejections() reads the flag as C++ JSG does (jsg::getCaptureThrowsAsRejections()), and Isolate::has_pending_exception() tells the helper that the body threw. The new promise tests cover each way a callback fails, with the flag on and off, a method that returns no promise, and a termination. Compatibility: no compatibility flag. No Rust API that returns a promise is exposed to Workers yet; this makes those to come match the C++ ones under the existing capture_async_api_throws flag.
Prepares for a Rust implementation of ExecutionContext (the `ctx` of a stateless entrypoint) that replaces the C++ one under an autogate. The runtime has to hold, pass and accept either, so: - exportedhandler::ExecutionContext (exported-handler-ctx.h) is the `ctx` a handler gets: the C++ api::ExecutionContext, or the JavaScript wrapper of a Rust one. It converts itself for JSG, and describes itself to RTTI as the C++ class, so the generated types don't change. It is named after the type users know, in a namespace of its own, so that conversion errors name the type as before, like messageevent::MessagePort. The header is temporary, until the C++ class is removed. - The handler signatures, ExportedHandler::ctx and getCtx(), Worker::ExecutionContextOrState, the queue and workflow entrypoints, tail streams and JS RPC use it. - Worker::Api::newExecutionContext() replaces wrapExecutionContext(): it creates the ctx rather than wrapping a C++ one, so that an implementation can create either. The default creates the C++ class, as before. Worker::Api::isRustExecutionContext() recognizes a Rust one when JavaScript passes a ctx back (super(ctx, env) in a WorkflowEntrypoint); the default returns false, so an embedder that doesn't link the Rust implementation needs nothing more. Nothing creates a Rust ExecutionContext yet, so behavior is unchanged. Compatibility: no compatibility flag; nothing observable changes.
src/workerd/api/execution-context/ is a Rust implementation of ExecutionContext, the `ctx` of a stateless entrypoint. With the rust-execution-context autogate enabled, WorkerdApi creates it instead of the C++ class, and recognizes it when JavaScript passes it back. It has every member of the C++ class, in the same order, with the same placement, compatibility flags, conversions and errors: - waitUntil(), passThroughOnException() and abort() call the Rust IoContext API, which calls what the C++ class calls. - props, exports and version are values it holds. - tracing, cache, access, mapVirtualHost() and restore() create or call the existing C++ objects (Tracing, CacheContext, AccessContext, Fetcher, restoreCurrentEntrypoint()) through jsg-cpp shims in ffi.c++. Tests: - ctx-shape-test compares what JavaScript observes of ctx: its tag, where each member lives and its attributes, enumeration, method names and lengths (also under spec_compliant_property_attributes), util.inspect(), conversion errors, mapVirtualHost() ports, the abort() errors, restore()'s errors with and without capture_async_api_throws, and a ctx used after its request. Its @all-autogates variant runs it against the Rust class; the existing ctx suites (persistent-stubs, js-rpc, ctx-access, server-test) do too. - execution-context-test checks that WorkerdApi creates the Rust class with the autogate enabled, and the C++ class without it. Compatibility: no compatibility flag. The Rust class is meant to be indistinguishable from the C++ one, which the tests above compare, and the autogate allows a rollback.
There was a problem hiding this comment.
Superseded by the review of
eb94483c.
This review, of 48583dce
✅ Approved — No correctness, security or performance problems found. The Rust ExecutionContext matches the C++ class member for member, it is gated behind an autogate, and the shape tests run against both implementations.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns. The access and cache properties still return undefined outside a request, as the C++ class does, and every C++ shim call goes through jsg_cpp::callCpp, which turns C++ exceptions into pending JS exceptions and throws nothing while execution is terminating.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
I compared the Rust ExecutionContext with the C++ api::ExecutionContext member by member: order, flag gating, lazy properties, version, cache and access outside a request, abort(), and restore() through restoreCurrentEntrypoint. They match. The Worker::Api plumbing and the getCtx(js) call sites are consistent, and the new shape tests run against both implementations.
⚡ Performance: ✅ LGTM
No performance concerns.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: 48583dce
There was a problem hiding this comment.
Superseded by the review of
52837d83.
This review, of eb94483c
✅ Approved — No correctness, security or performance problems found. The Rust ExecutionContext matches the C++ class member for member, it is gated behind an autogate, and the shape tests run against both implementations.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns. The access and cache properties still return undefined outside a request, as the C++ class does, and every C++ shim call goes through jsg_cpp::callCpp, which turns C++ exceptions into pending JS exceptions and throws nothing while execution is terminating.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
I compared the Rust ExecutionContext with the C++ api::ExecutionContext member by member: order, flag gating, lazy properties, version, cache and access outside a request, abort(), and restore() through restoreCurrentEntrypoint. They match. The Worker::Api plumbing and the getCtx(js) call sites are consistent, and the new shape tests run against both implementations.
⚡ Performance: ✅ LGTM
No performance concerns.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: eb94483c — same changes as 48583dce, so this is that commit's review (no new review was run; line numbers are as of 48583dce)
There was a problem hiding this comment.
Superseded by the review of
c6f615ce.
This review, of 52837d83
✅ Approved — No correctness, security or performance problems found. The Rust ExecutionContext matches the C++ class member for member, it is gated behind an autogate, and the shape tests run against both implementations.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns. The access and cache properties still return undefined outside a request, as the C++ class does, and every C++ shim call goes through jsg_cpp::callCpp, which turns C++ exceptions into pending JS exceptions and throws nothing while execution is terminating.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
I compared the Rust ExecutionContext with the C++ api::ExecutionContext member by member: order, flag gating, lazy properties, version, cache and access outside a request, abort(), and restore() through restoreCurrentEntrypoint. They match. The Worker::Api plumbing and the getCtx(js) call sites are consistent, and the new shape tests run against both implementations.
⚡ Performance: ✅ LGTM
No performance concerns.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: 52837d83 — same changes as 48583dce, so this is that commit's review (no new review was run; line numbers are as of 48583dce)
There was a problem hiding this comment.
Superseded by the review of
8df89628.
This review, of c6f615ce
✅ Approved — No correctness, security or performance problems found. The Rust ExecutionContext matches the C++ class member for member, it is gated behind an autogate, and the shape tests run against both implementations.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns. The access and cache properties still return undefined outside a request, as the C++ class does, and every C++ shim call goes through jsg_cpp::callCpp, which turns C++ exceptions into pending JS exceptions and throws nothing while execution is terminating.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
I compared the Rust ExecutionContext with the C++ api::ExecutionContext member by member: order, flag gating, lazy properties, version, cache and access outside a request, abort(), and restore() through restoreCurrentEntrypoint. They match. The Worker::Api plumbing and the getCtx(js) call sites are consistent, and the new shape tests run against both implementations.
⚡ Performance: ✅ LGTM
No performance concerns.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: c6f615ce — same changes as 48583dce, so this is that commit's review (no new review was run; line numbers are as of 48583dce)
There was a problem hiding this comment.
Superseded by the review of
95a27d08.
This review, of 8df89628
✅ Approved — No correctness, security or performance problems found. The Rust ExecutionContext matches the C++ class member for member, it is gated behind an autogate, and the shape tests run against both implementations.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns. The access and cache properties still return undefined outside a request, as the C++ class does, and every C++ shim call goes through jsg_cpp::callCpp, which turns C++ exceptions into pending JS exceptions and throws nothing while execution is terminating.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
I compared the Rust ExecutionContext with the C++ api::ExecutionContext member by member: order, flag gating, lazy properties, version, cache and access outside a request, abort(), and restore() through restoreCurrentEntrypoint. They match. The Worker::Api plumbing and the getCtx(js) call sites are consistent, and the new shape tests run against both implementations.
⚡ Performance: ✅ LGTM
No performance concerns.
📝 Documentation: ✅ LGTM
No documentation issues.
Reviewed commit: 8df89628 — same changes as 48583dce, so this is that commit's review (no new review was run; line numbers are as of 48583dce)
guybedford
left a comment
There was a problem hiding this comment.
LGTM, just a couple of stale references inline.
…ion-context # Conflicts: # src/workerd/server/BUILD.bazel # src/workerd/server/workerd-api.c++ # src/workerd/server/workerd-api.h # src/workerd/util/autogate.h
The comments of the execution-context package named RustExecutionContextTag::isInstance() and api::newExecutionContext(), which do not exist. The C++ functions that create and recognize a Rust ExecutionContext are api::newRustExecutionContext() and api::isRustExecutionContext().
There was a problem hiding this comment.
Superseded by the review of
af272ed9.
This review, of 95a27d08
✅ Approved — No issues found: the stale comment names from the earlier discussion are fixed, and the merge from main keeps the autogate and the WorkerdApi plumbing consistent.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
The comments now name api::newRustExecutionContext() and api::isRustExecutionContext(), which exist in execution-context.h and ffi.c++. The merge-related changes are consistent:
server/BUILD.bazeldepends on both the Rust crate and the:ffishims.- The
WorkerdApioverrides match theWorker::Apivirtuals. isRustExecutionContext()deliberately does not check the autogate, so a Rust ctx that JS passes back is still recognised.- The autogate entry is well-formed.
⚡ Performance: ✅ LGTM
No performance concerns.
getCtx(js)adds a constant-timeaddRefand wrap per handler invocation.- The Rust allocation replaces the C++ one only when the autogate is on.
📝 Documentation: ✅ LGTM
The stale function names that came up in the discussion are corrected.
Reviewed commit: 95a27d08
There was a problem hiding this comment.
Superseded by the review of
69e86218.
This review, of af272ed9
✅ Approved — No issues found: the stale comment names from the earlier discussion are fixed, and the merge from main keeps the autogate and the WorkerdApi plumbing consistent.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
The comments now name api::newRustExecutionContext() and api::isRustExecutionContext(), which exist in execution-context.h and ffi.c++. The merge-related changes are consistent:
server/BUILD.bazeldepends on both the Rust crate and the:ffishims.- The
WorkerdApioverrides match theWorker::Apivirtuals. isRustExecutionContext()deliberately does not check the autogate, so a Rust ctx that JS passes back is still recognised.- The autogate entry is well-formed.
⚡ Performance: ✅ LGTM
No performance concerns.
getCtx(js)adds a constant-timeaddRefand wrap per handler invocation.- The Rust allocation replaces the C++ one only when the autogate is on.
📝 Documentation: ✅ LGTM
The stale function names that came up in the discussion are corrected.
Reviewed commit: af272ed9 — same changes as 95a27d08, so this is that commit's review (no new review was run; line numbers are as of 95a27d08)
There was a problem hiding this comment.
✅ Approved — No issues found: the stale comment names from the earlier discussion are fixed, and the merge from main keeps the autogate and the WorkerdApi plumbing consistent.
The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.
✅ All checks passed
🔒 Security: ✅ LGTM
No security concerns.
📖 AGENTS.md: ✅ LGTM
No AGENTS.md issues.
📋 Code Quality: ✅ LGTM
The comments now name api::newRustExecutionContext() and api::isRustExecutionContext(), which exist in execution-context.h and ffi.c++. The merge-related changes are consistent:
server/BUILD.bazeldepends on both the Rust crate and the:ffishims.- The
WorkerdApioverrides match theWorker::Apivirtuals. isRustExecutionContext()deliberately does not check the autogate, so a Rust ctx that JS passes back is still recognised.- The autogate entry is well-formed.
⚡ Performance: ✅ LGTM
No performance concerns.
getCtx(js)adds a constant-timeaddRefand wrap per handler invocation.- The Rust allocation replaces the C++ one only when the autogate is on.
📝 Documentation: ✅ LGTM
The stale function names that came up in the discussion are corrected.
Reviewed commit: 69e86218 — same changes as 95a27d08, so this is that commit's review (no new review was run; line numbers are as of 95a27d08)
Port
ExecutionContext(thectxof a stateless entrypoint) to RustJSG behind a new
rust-execution-contextautogate.The first part lets
Worker::Apichoose the ExecutionContext ahandler gets. It is C++ plumbing with no behaviour change.
exportedhandler::ExecutionContext(exported-handler-ctx.h) is thectxa handler gets: either the C++api::ExecutionContextor the JSwrapper of a Rust one. It converts itself for JSG and uses
JsgRttiDelegate, so the generated TypeScript types don't change; itfollows the pattern of
messageevent::MessagePort, and the header goesaway with the C++ class. Handler signatures,
ExportedHandler::ctx/getCtx(),ExecutionContextOrState, queue,WorkflowEntrypoint,tail streams and JS RPC use the new type.
Worker::Api::newExecutionContext()replaceswrapExecutionContext();the default creates the C++ class.
Worker::Api::isRustExecutionContext()recognises a Rust ctx that JSpasses back (
super(ctx, env)). It returns false by default, so anembedder that doesn't link the crate needs nothing more.
The second part adds the Rust implementation in
src/workerd/api/execution-context/. It has every member of the C++class, in the same order, with the same placement, flag gating and
errors.
waitUntil(),passThroughOnException()andabort()gothrough the Rust IoContext API;
props,exportsandversionarevalues it holds;
tracing,cache,access,mapVirtualHost()andrestore()call the existing C++ objects through jsg-cpp shims.restore()relies on a Rust method's returned promise being rejectedunder
capture_async_api_throws(#8255).WorkerdApicreates the Rustclass when the autogate is on.
ctx-shape-testchecks what JS observes ofctx. Its@all-autogatesvariant runs against the Rust class and the other variants against the
C++ class, with identical expectations. It covers the tag, member
placement and attributes, enumeration order, method names and lengths
(also under
spec_compliant_property_attributes),util.inspect(),conversion errors,
mapVirtualHost()port handling, theabort()errors,
restore()errors with and withoutcapture_async_api_throws,and a ctx used after its request has ended.
execution-context-testchecks that
WorkerdApicreates the Rust class with the gate on andthe C++ class with it off. The existing ctx suites (persistent-stubs,
js-rpc, ctx-access, server-test) also pass under
@all-autogates.There is no compatibility flag. The Rust class is meant to be
indistinguishable from the C++ one, which the tests above compare, and
the autogate allows a rollback.