Repository navigation
fix(sdk-build): build embedded Python/Node SDKs with panic=unwind - #3783
Conversation
The Python and Node SDKs load the engine as a native library inside the user's own process. Under the release profile's panic="abort", a raw Rust panic anywhere in the engine (e.g. a stdlib slice bounds check) calls process::abort() and SIGABRTs the user's interpreter — their whole server dies, uncatchable even by `except BaseException`. The bridges already have the machinery to contain such a panic (a catch_unwind -> baml.panics.SdkPanic boundary in bridge_cffi, and tokio's per-task isolation in the in-engine HTTP server), but it only runs when the stack can unwind, which abort forbids — so it was dead code in shipped wheels. Add release-bridge-python / release-bridge-node profiles (release + panic= unwind) and point the maturin and napi release builds at them. An engine panic now surfaces as a catchable BamlPanic / JS error instead of killing the host. The CLI and wasm stay on release/abort: each owns its own process and is size-gated, so the ~5MB of unwind tables isn't worth paying there. Verified: a BAML http.Server handler panic (the bex_str SWAR OOB) takes down the whole process under abort (exit 134) but is isolated under unwind (server keeps serving); and calling the same panicking function from Python through the unwind .so raises a catchable BamlPanic and the process survives (exit 0).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTwo new Cargo profiles ( ChangesBridge SDK panic=unwind profiles
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
No description provided. |
Problem
The Python and Node SDKs load the engine as a native library inside the user's own process (
baml_py.abi3.sovia PyO3, the.nodeaddon via napi). The release profile setspanic = "abort"(a binary-size win from #3110). So a raw Rust panic anywhere in the engine — e.g. a stdlib slice bounds check — callsprocess::abort()and SIGABRTs the user's interpreter: their whole server dies, exit 134, uncatchable even byexcept BaseException.The bridges already have machinery to contain such a panic — a
catch_unwind → baml.panics.SdkPanicboundary inbridge_cffi, plus tokio's per-task isolation in the in-engine HTTP server — but all of it only runs when the stack can unwind, whichabortforbids. It was effectively dead code in shipped wheels/addons.Fix
Add
release-bridge-python/release-bridge-nodeprofiles (inherits = "release",panic = "unwind") and point the maturin and napi release builds at them.panicis a whole-artifact setting (can't be per-crate, can't mix strategies in one binary), so the embedded SDKs get their own profile selected at build time.The CLI and wasm stay on
release/abort: each owns its own process and is size-gated, so an abort there is acceptable and the unwind tables aren't worth paying for.Verification
bex_strSWAR OOB, fully BAML-triggerable): underabortone bad request takes down the whole server process (exit 134); underunwindthe panic is isolated,/healthzstill returns 200, process survives.release-bridge-python.soraises a catchablebaml_core.errors.BamlPanicand the process survives (exit 0), instead of SIGABRT. Confirmed the built.sois unwind (__eh_frame1.24 MB vs ~55 KB for abort).Cost / notes
.so13.2→18.3 MB). Not tracked by the size gate (which gates onlybaml-cli/packed-program/wasm).--profilerequires@napi-rs/cliv3 — pinned at3.7.0.maturin develop,pnpm build:debug) already used thedevprofile (unwind); only shipped artifacts wereabort.Summary by CodeRabbit
Bug Fixes
Chores