Repository navigation
Conversation
JsRpcTargetBase::callImpl() made an actor's RPC result wait for the output gate, but not its exception. A method could throw an error carrying state whose write was not yet confirmed, and the caller saw it even if that write then failed. WorkerEntrypoint::request() already gates a fetch handler's exceptions. Wait for the gate on rejection too, and turn a synchronous JS throw into a rejection so it takes the same path. KJ exceptions from decoding the arguments propagate unchanged. No compatibility flag: only the timing of an error changes, as fetch already does.
|
All contributors have signed the CLA ✍️ ✅ |
ry
changed the title
|
I have read the CLA Document and I hereby sign the CLA |
JSG_CATCH handles the termination check and releases its TryCatch before the handler runs, replacing the hand-written v8::TryCatch. A KJ exception from decoding the arguments is rethrown ahead of JSG_CATCH, so it still propagates unchanged rather than being converted to JavaScript.
| } catch (kj::Exception&) { | ||
| throw; | ||
| } |
There was a problem hiding this comment.
The JSG_CATCH should cover this (the catch block for kj::Exception&) should not be needed.
But also, just as a general point, we don't use the bare throw;. When this is needed we would use kj::throwFatalException(...).
There was a problem hiding this comment.
The kj::Exception clause is needed. JSG_CATCH is catch (...), and it converts a kj::Exception to JS through exceptionToJs(). A KJ exception here comes from decoding the arguments, not from the method, so it has to propagate unchanged. If I remove the clause, jsrpc-call-plan-test.c++:1216 fails because the caller gets jsg.Error: internal error; reference = ... instead of the original exception.
I've switched the bare throw; to kj::throwFatalException(kj::mv(e)).
2 participants
JsRpcTargetBase::callImpl()made an actor's RPC result wait for the output gate, but not its exception. A method could throw an error carrying state whose write was not yet confirmed, and the caller saw it even if that write then failed.WorkerEntrypoint::request()already gates a fetch handler's exceptions.Wait for the gate on rejection too, and turn a synchronous JS throw into a rejection so it takes the same path. KJ exceptions from decoding the arguments propagate unchanged.
No compatibility flag: only the timing of an error changes, as fetch already does.