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

Sjors referenced this in commit 7a6d210989 on Apr 7, 2026

alfonsoromanz referenced this in commit b922a4ea40 on Apr 7, 2026

alfonsoromanz referenced this in commit 04b9193497 on Apr 7, 2026

alfonsoromanz referenced this in commit 30eda85827 on Apr 7, 2026

alfonsoromanz referenced this in commit c7961f8268 on Apr 7, 2026

alfonsoromanz referenced this in commit 42838a0929 on Apr 7, 2026

alfonsoromanz referenced this in commit 1a3de77a5c on Apr 7, 2026

achow101 referenced this in commit 1d7edee34c on Apr 7, 2026

alfonsoromanz referenced this in commit b555a0b789 on Apr 7, 2026

achow101 referenced this in commit d2844c6a4f on Apr 7, 2026

fanquake referenced this in commit 1bc22a38a8 on Apr 8, 2026

fanquake referenced this in commit afa8ba04e0 on Apr 8, 2026

Sjors referenced this in commit 65abfa3c1a on Apr 15, 2026

morozow referenced this in commit fa2e74308e on May 8, 2026

Sjors referenced this in commit 0dd3f5c989 on May 12, 2026