Conversation
Contributor
|
I'm Bonk, and I've done a quick review of your PR. Adds Rust worker pending startup and C++ bridge support.
Posted 3 inline review comments. |
danlapid
force-pushed
the
dlapid/rs-worker-crate
branch
2 times, most recently
from
September 26, 2026 23:01
187e172 to
85207d2
Compare
Contributor
|
Since last review: 0 resolved, 0 still open, 0 new. Reviewed commit: 832afcb4 · github run |
danlapid
force-pushed
the
dlapid/rs-worker-crate
branch
from
September 26, 2026 23:14
85207d2 to
56e2a43
Compare
danlapid
force-pushed
the
dlapid/rs-worker-crate
branch
from
September 27, 2026 00:18
56e2a43 to
25ea59d
Compare
Adds to the `worker` crate the pieces a Rust caller needs to stand in for, and hand off to, a C++ `workerd::WorkerInterface`: - `PromisedInterface`: an `Interface` whose target is still starting. It holds the future that produces the target's `WorkerInterface`; the first event drives that future and every event, first or later, waits for the target and then runs on it. A start failure is kept as the complete `KjError` (type, description, throw site and detail records, which is how CPU, memory, wall-time and kill-switch failures are classified) and every later event fails with a clone of it. C++ constructs one through `new_promised_interface` / `newPromisedInterface(WorkerPromise)`, the counterpart of `newPromisedWorkerInterface`. - `Interface::abandon_alarm`, with the same no-op default as `WorkerInterface::abandonAlarm`, bridged in both directions (`RustWorkerInterface::abandonAlarm` -> `Wrapper::abandon_alarm`, and `CxxWorkerInterface::abandon_alarm` -> `worker_abandon_alarm`). The stored alarm time crosses as i64 nanoseconds like the other dates. `PromisedInterface` delegates it to its target once started, as `PromisedWorkerInterface` does, so an actor reached through a Rust interface still clears its alarm state when retries are exhausted. - `Interface::into_kj()`: converts a Rust `Interface` into the `kj::Own<WorkerInterface>` that C++ expects, through the `wrapper_into_kj` bridge function (`kj::from<kj_rs::Rust>` over the `Wrapper` box in ffi.c++). - `not_supported(event)` and `Interface::custom_event`'s default body: answer a custom event the way a C++ `WorkerInterface` that returns `event->notSupported()` does, so implementations that do not handle custom events no longer have to answer for the event themselves. The `ok` test worker drops its `custom_event` override accordingly, and a KJ_TEST checks the default defers to the event's `notSupported()`. - `custom_event_failed(event, error)`: delivers a start failure to a custom event that will never run (`event->failed(...)`), so a JS RPC session resolves its client to the original error rather than reporting the event destroyed before completion. The error crosses as a boxed Rust `Error` whose `raise()` returns it as `Err`; the bridge's own `Result` path throws that as the equivalent `kj::Exception`, so the event receives the full exception (location and detail records included, which `JsRpcSessionCustomEvent::failed()` relies on) rather than one rebuilt from type and text. `PromisedInterface` uses it when its target fails to start. `toRustOutcome` moves from ffi.c++ into bridge.h alongside its inverse `fromImpl`, `:bridge` becomes publicly visible, and the crate gains the `futures` dependency for `LocalBoxFuture`. The `custom_event_failed` shim that delivers that failure is declared fallible (`-> Result<()>`): `CustomEvent::failed()` is overridable and may throw, and an `extern "C++"` shim that can throw must return `Result` so the exception reaches Rust as an error rather than a panic at the bridge. `PromisedInterface::custom_event` propagates it, so a throwing `failed()` rejects the event with that exception. The other reverse shims are all `async -> Result<..>` already, and `wrapper_into_kj` only heap-allocates. KJ_TESTs cover the startup failure surfacing in full on repeated later events and through `customEvent`'s `failed()`, a `failed()` that throws rejecting the `customEvent` promise with its exception, and `abandonAlarm` delegation through `PromisedInterface` both before and after the target starts. These are used by the Rust server in a following PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
danlapid
force-pushed
the
dlapid/rs-worker-crate
branch
from
September 27, 2026 02:07
25ea59d to
832afcb
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
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.
Adds to the
workercrate the pieces a Rust caller needs to stand infor, and hand off to, a C++
workerd::WorkerInterface:PromisedInterface: anInterfacewhose target is still starting. Itholds the future that produces the target's
WorkerInterface; thefirst event drives that future and every event, first or later, waits
for the target and then runs on it. A start failure is kept as the
complete
KjError(type, description, throw site and detail records,which is how CPU, memory, wall-time and kill-switch failures are
classified) and every later event fails with a clone of it. C++
constructs one through
new_promised_interface/newPromisedInterface(WorkerPromise), the counterpart ofnewPromisedWorkerInterface.Interface::abandon_alarm, with the same no-op default asWorkerInterface::abandonAlarm, bridged in both directions(
RustWorkerInterface::abandonAlarm->Wrapper::abandon_alarm, andCxxWorkerInterface::abandon_alarm->worker_abandon_alarm). Thestored alarm time crosses as i64 nanoseconds like the other dates.
PromisedInterfacedelegates it to its target once started, asPromisedWorkerInterfacedoes, so an actor reached through a Rustinterface still clears its alarm state when retries are exhausted.
Interface::into_kj(): converts a RustInterfaceinto thekj::Own<WorkerInterface>that C++ expects, through thewrapper_into_kjbridge function (kj::from<kj_rs::Rust>over theWrapperbox in ffi.c++).not_supported(event)andInterface::custom_event's default body:answer a custom event the way a C++
WorkerInterfacethat returnsevent->notSupported()does, so implementations that do not handlecustom events no longer have to answer for the event themselves. The
oktest worker drops itscustom_eventoverride accordingly, and aKJ_TEST checks the default defers to the event's
notSupported().custom_event_failed(event, error): delivers a start failure to acustom event that will never run (
event->failed(...)), so a JS RPCsession resolves its client to the original error rather than
reporting the event destroyed before completion. The error crosses as
a boxed Rust
Errorwhoseraise()returns it asErr; the bridge'sown
Resultpath throws that as the equivalentkj::Exception, sothe event receives the full exception (location and detail records
included, which
JsRpcSessionCustomEvent::failed()relies on) ratherthan one rebuilt from type and text.
PromisedInterfaceuses it whenits target fails to start.
toRustOutcomemoves from ffi.c++ into bridge.h alongside its inversefromImpl,:bridgebecomes publicly visible, and the crate gains thefuturesdependency forLocalBoxFuture.The
custom_event_failedshim that delivers that failure is declaredfallible (
-> Result<()>):CustomEvent::failed()is overridable andmay throw, and an
extern "C++"shim that can throw must returnResultso the exception reaches Rust as an error rather than a panicat the bridge.
PromisedInterface::custom_eventpropagates it, so athrowing
failed()rejects the event with that exception. The otherreverse shims are all
async -> Result<..>already, andwrapper_into_kjonly heap-allocates.KJ_TESTs cover the startup failure surfacing in full on repeated later
events and through
customEvent'sfailed(), afailed()that throwsrejecting the
customEventpromise with its exception, andabandonAlarmdelegation throughPromisedInterfaceboth before andafter the target starts.
These are used by the Rust server in a following PR.
🤖 Generated with Claude Code