Use throwRecoverableException instead of raw throw for stored exceptions #268

alfonsoromanz commented at 6:26 PM on April 6, 2026: contributor

Replace throw kj::mv(*exception) with kj::throwRecoverableException(kj::mv(*exception)) in the async worker thread exception handler in type-context.h.

A raw throw creates a kj::Exception with the calling binary's typeinfo. On macOS with -fvisibility=hidden, this typeinfo is not coalesced with libkj.dylib's copy by the dynamic linker, so getCaughtExceptionAsKj()'s catch (kj::Exception& e) fails and falls through to catch (...), producing unknown non-KJ exception of type: kj::Exception errors.

throwFatalException() routes the throw through libkj, wrapping it in ExceptionImpl with consistent typeinfo. This matches the pattern KJ uses internally in kj/async-inl.h when re-throwing stored exceptions from promises.

Context: bitcoin/bitcoin#34723, bitcoin/bitcoin#35014

DrahtBot commented at 6:27 PM on April 6, 2026: none

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

in include/mp/type-context.h:192 in 1b09eb5e94

 188 | @@ -189,7 +189,7 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
 189 |              }
 190 |          })) {
 191 |              MP_LOG(loop, Log::Error) << "IPC server request #" << req << " uncaught exception (" << kj::str(*exception).cStr() << ")";
 192 | -            throw kj::mv(*exception);
 193 | +            kj::throwFatalException(kj::mv(*exception));

ryanofsky commented at 7:01 PM on April 6, 2026:

In commit "Use throwFatalException instead of raw throw for stored exceptions" (1b09eb5e94149091749bf07c44f894832e9cad02)

I think it would probably be more appropriate to use throwRecoverableException instead of throwFatalException here assuming either of these would fix the problem on macos. Currently exceptions are caught and thrown here when cilents make invalid requests, and even though these are typically fatal errors from the client point of view, they should be recoverable from the server point of view.

alfonsoromanz commented at 8:08 PM on April 6, 2026:

Makes sense, updated to throwRecoverableException (23be44b0d30514aa82fb539925b8650478d03bd9) . Also pushed the same change to bitcoin/bitcoin#35014 where CI passes (except the subtree lint, as expected).

ryanofsky commented at 7:01 PM on April 6, 2026: collaborator

Code review 1b09eb5e94149091749bf07c44f894832e9cad02

Use throwRecoverableException instead of raw throw for stored exceptions

Replace `throw kj::mv(*exception)` with `kj::throwRecoverableException(kj::mv(*exception))` in the async worker thread exception handler.

A raw throw creates a kj::Exception with the calling binary's typeinfo. On macOS with -fvisibility=hidden, this typeinfo is not coalesced with libkj.dylib's copy by the dynamic linker, so getCaughtExceptionAsKj()'s catch (kj::Exception& e) fails and falls through to catch (...), producing "unknown non-KJ exception of type: kj::Exception" errors.

throwRecoverableException() routes the throw through libkj, wrapping it in ExceptionImpl with consistent typeinfo. This matches the pattern KJ uses internally in kj/async-inl.h when re-throwing stored exceptions from promises.

23be44b0d3

alfonsoromanz force-pushed on Apr 6, 2026

alfonsoromanz renamed this:
~~Use throwFatalException instead of raw throw for stored exceptions~~
Use throwRecoverableException instead of raw throw for stored exceptions
on Apr 6, 2026

ryanofsky approved

ryanofsky commented at 9:00 PM on April 6, 2026: collaborator

Code review ACK 23be44b0d30514aa82fb539925b8650478d03bd9. Thanks for the fix!

Sjors commented at 1:26 PM on April 7, 2026: member

ACK 23be44b0d30514aa82fb539925b8650478d03bd9

It gets rid of the macOS test workaround, as shown by https://github.com/bitcoin/bitcoin/pull/35014.

The capnproto style guide also speaks of recoverable exceptions: https://github.com/capnproto/capnproto/blob/v2/style-guide.md#exceptions-can-happen-anywhere-including-destructors

ryanofsky commented at 1:52 PM on April 7, 2026: collaborator

re: #268 (comment)

The capnproto style guide also speaks of recoverable exceptions: https://github.com/capnproto/capnproto/blob/v2/style-guide.md#exceptions-can-happen-anywhere-including-destructors

Interesting, I wasn't sure what "recoverable" was supposed to mean other than non-fatal, but the document refers to:

"recoverable" exceptions, where it is safe to continue execution without throwing in cases where throwing would be bad.

So I guess another aspect of recoverable exceptions is that they are allowed to be suppressed in destructors when there is already another exception active.

ryanofsky merged this on Apr 7, 2026

ryanofsky closed this on Apr 7, 2026

alfonsoromanz referenced this in commit 486bfdfebe on Apr 7, 2026

alfonsoromanz referenced this in commit dbeacebb9b on Apr 7, 2026