Repository navigation
Reject a Rust method's returned promise with what it throws - #8255
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.
There was a problem hiding this comment.
Superseded by the review of
2ae33ceb.
This review, of 9c03481f
✅ Approved — No findings. With the flag on, a promise-returning Rust callback now rejects its promise with what it throws, as C++ JSG does. Terminations still propagate, and nothing changes with the flag off.
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 concerns.
📋 Code Quality: ✅ LGTM
No problems found.
- A termination is thrown again and keeps propagating.
- The rejected promise is created only after the
TryCatchhas gone out of scope. - The generated callbacks' early returns behave the same as before.
- Tests cover the flag-on, flag-off and termination paths.
⚡ Performance: ✅ LGTM
The added work is one flag read per call, and only on methods that return a promise. try_catch runs only when the flag is on.
📝 Documentation: ✅ LGTM
The new README bullet correctly describes how promise-returning callbacks behave under capture_async_api_throws. It also narrows the general statement just above it that a Result error throws.
Reviewed commit: 9c03481f
There was a problem hiding this comment.
✅ Approved — No findings. With the flag on, a promise-returning Rust callback now rejects its promise with what it throws, as C++ JSG does. Terminations still propagate, and nothing changes with the flag off.
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 concerns.
📋 Code Quality: ✅ LGTM
No problems found.
- A termination is thrown again and keeps propagating.
- The rejected promise is created only after the
TryCatchhas gone out of scope. - The generated callbacks' early returns behave the same as before.
- Tests cover the flag-on, flag-off and termination paths.
⚡ Performance: ✅ LGTM
The added work is one flag read per call, and only on methods that return a promise. try_catch runs only when the flag is on.
📝 Documentation: ✅ LGTM
The new README bullet correctly describes how promise-returning callbacks behave under capture_async_api_throws. It also narrows the general statement just above it that a Result error throws.
Reviewed commit: 2ae33ceb — same changes as 9c03481f, so this is that commit's review (no new review was run; line numbers are as of 9c03481f)
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::Localv8::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
ctx.restore(42).catch(...).The generated callback of a #[jsg_method], #[jsg_property] getter or
#[jsg_callable] that returns a jsg::Promise or a v8::Localv8::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 jsg-macros README documents the behavior.
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.
Without the macro change, the test with the flag on fails.
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.