bun:ffi: don't re-throw JSC's TerminationException from a JSCallback - #32792
Merged
Claude / Claude Code Review
completed
Jun 27, 2026 in 30m 1s
Code review found 2 potential issues
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 2 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟣 Pre-existing | src/jsc/bindings/JSFFIFunction.cpp:218-221 |
scriptExecutionStatus guard is currently inert — Bun__VM__scriptExecutionStatus stub always returns Running |
| 🟣 Pre-existing | src/jsc/bindings/JSFFIFunction.cpp:215-216 |
[pre-existing] 'Threadsafe functions must return void' validation is dead code |
Annotations
Check notice on line 221 in src/jsc/bindings/JSFFIFunction.cpp
claude / Claude Code Review
scriptExecutionStatus guard is currently inert — Bun__VM__scriptExecutionStatus stub always returns Running
🔵 pre-existing: this guard is currently inert — `Zig::GlobalObject::scriptExecutionStatus` calls extern-C `Bun__VM__scriptExecutionStatus`, whose only linked definition is the Phase-C stub at `src/bun_bin/phase_c_exports.rs:134-138` that unconditionally returns `0` (Running); the real `VirtualMachine::script_execution_status()` is never exported despite its doc comment. So the early return can never fire (same for the `MessagePort::dispatchOneMessage` pattern this copies). Harmless in practice —
Check notice on line 216 in src/jsc/bindings/JSFFIFunction.cpp
claude / Claude Code Review
[pre-existing] 'Threadsafe functions must return void' validation is dead code
Drive-by, **pre-existing** (not introduced by this PR, exists identically on main) — surfaced while tracing how `{ threadsafe: true }` reaches `FFI_Callback_threadsafe_call`. In `generate_symbol_for_function` at `src/runtime/ffi/ffi_body.rs:1819` and `src/runtime/ffi/host_fns.rs:126`, the "Threadsafe functions must return void" check reads `function.threadsafe` (the out-param's field) instead of the local `threadsafe` that was just parsed; the field isn't set until after the check (and after `*f
Loading