Capture stack traces in thrown errors with source line numbers#3339
Merged
antoniosarosi merged 10 commits intocanaryfrom Apr 12, 2026
Merged
Capture stack traces in thrown errors with source line numbers#3339antoniosarosi merged 10 commits intocanaryfrom
antoniosarosi merged 10 commits intocanaryfrom
Conversation
…numbers Thread file paths into Function, fix closure bug via as_callable(), capture call stacks at throw time before frame unwinding, and propagate structured Vec<ErrorLocation> traces through VmError and EngineError. Populate bytecode line tables by setting MIR source spans in the lowering pass. Unhandled throws now include Python-style tracebacks with file paths and line numbers in their Display output, automatically surfaced through all FFI bridges.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Binary size checks passed✅ 7 passed
Generated by |
Replace manual frame-by-frame assertions with insta::assert_snapshot! for clearer, more maintainable stack trace test output.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThreads source-file/span into MIR→bytecode emission, captures VM stack traces during unwinding, propagates traces through bytecode exception tables and engine errors, adds BAML Changes
Sequence DiagramsequenceDiagram
participant Caller as Compiler/Emitter
participant VM as VM Runtime
participant Frames as Frame Stack
participant Trace as Trace Vec
participant Handler as Catch Handler
Caller->>VM: execute bytecode (user function)
VM->>Frames: maintain frames (function, ip, source_file)
Frames->>VM: on throw -> VM captures current frames
VM->>Trace: map frames -> StackFrame(file_path,line,function)
VM->>Handler: unwind, locate exception table entry
alt entry.has_stack_trace_slot
VM->>VM: alloc_stack_trace(trace) -> StackTrace Value
VM->>Handler: store error and stack-trace value into locals
else
VM->>Handler: store error only
end
Handler->>Caller: execute catch body with bound locals
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
baml_language/crates/bex_vm/src/vm.rs (1)
1038-1114:⚠️ Potential issue | 🟠 MajorPreserve traceback state across rethrows instead of rebuilding it.
This trace is computed before the first unwind, but it is discarded as soon as a handler is found. The compiler-generated rethrows on unmatched arms and
throw_if_panicthen rebuild from the already-popped stack, so the original callee frames disappear and the reported line becomes the synthetic rethrow site. Cases likewrong_panic_pattern_propagateswill surface truncated tracebacks unless the captured trace travels with the exception and is reused on rethrow.baml_language/crates/baml_compiler2_emit/src/lib.rs (1)
1061-1122:⚠️ Potential issue | 🟠 MajorPopulate
source_filefor compiled lambdas.Unlike the top-level bytecode path at Line 437 and the
$init_let_*helper path at Line 1188, this branch only setsf.name. Every user-authored lambda therefore keeps the default emptysource_file, so traceback frames that come from closures lose their filename even though this PR is supposed to surface file/line information through closures too.Suggested fix
fn compile_lambdas_flat( lambdas: &[baml_compiler2_mir::MirFunction], line_starts: &[u32], + source_file: &str, globals: &HashMap<String, usize>, classes: &HashMap<String, HashMap<String, usize>>, class_object_indices: &HashMap<String, usize>, @@ let nested_info = compile_lambdas_flat( &lambda.lambdas, line_starts, + source_file, globals, classes, class_object_indices, @@ let mut f = compile_mir_function( body, lambda.arity, lambda.span, line_starts, ctx, OptLevel::One, ); f.name.clone_from(&lambda_name); + f.source_file = source_file.to_string(); let idx = objects.len(); objects.push(Object::Function(Box::new(f)));Both callers can pass the same per-file path they already compute for top-level functions /
$init_let_*helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e6b3d4e-ac35-4078-aeb7-56f4aa5e0fa0
⛔ Files ignored due to path filters (2)
baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
baml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_tests/tests/errors.rsbaml_language/crates/baml_tests/tests/exceptions.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/lib.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/types.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/src/types.rs
Comment on lines
+915
to
+919
| // Set the function-level span on the builder so MirFunction::span is populated. | ||
| self.builder.set_span(baml_base::Span::new( | ||
| self.file.file_id(self.db), | ||
| func_data.span, | ||
| )); |
Contributor
There was a problem hiding this comment.
Also seed MirFunction::span for lambdas.
Line 915 initializes the top-level builder span, but lower_lambda() still creates a fresh MirBuilder without calling set_span. That leaves closure MirFunctions with empty function-level metadata even after this PR starts threading callable source locations into diagnostics.
📍 Suggested follow-up
// in lower_lambda(...)
self.builder = MirBuilder::new(Name::new(&lambda_name), arity);
+if let Some(span) = self.span_for_expr(expr_id) {
+ self.builder.set_span(span);
+}
Comment on lines
+43
to
+48
| Object::Closure(closure) => { | ||
| // SAFETY: closure.function points to a Function object that lives | ||
| // for the lifetime of the program (stored in the object pool). | ||
| // Same guarantee as in load_function() in vm.rs. | ||
| let func_obj: &Object = unsafe { closure.function.get() }; | ||
| func_obj.as_function() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 '\bstruct HeapPtr\b|\bfn get\s*\(' --glob '*.rs'
rg -n -C3 'load_function|closure\.function\.get\(' --glob '*.rs'
rg -n -C3 '\bapply_forwarding\b|\bforwarding\b' --glob '*.rs'Repository: BoundaryML/baml
Length of output: 50371
Tighten the safety comment to match the actual contract.
The current safety comment claims closure.function "lives for the lifetime of the program," but this overstates the guarantee. The HeapPtr::get() contract requires only pointer validity and no concurrent writes, not a 'static lifetime. In reality, closure function objects are subject to GC and their pointers are updated via the forwarding map (see fixup_object_references in gc.rs:378). The dereference is safe here because it occurs during VM frame execution, where frame pointers are kept valid by apply_frame_forwarding() after each GC. The safety comment should reference this forwarding-based validity rather than claiming program-lifetime storage.
Resolve conflicts between stack trace capture feature and canary changes:
- Adapt to VmError::Thrown(Value) tuple variant (canary renamed from UnhandledThrow)
- Add VmError::ThrownUnhandled variant for terminal throws with trace data
- Keep VmInternalError return types on ObjectTrait methods
- Update EngineError::UnhandledThrow match sites to use { value, .. } pattern
- Regenerate bytecode format snapshots
- Add stack_trace.baml with StackFrame (file, line, function_name) and StackTrace (frames, to_string method) classes - Implement BamlClassErrorsStackTrace::to_string in stack_trace.rs - Add alloc_stack_trace helper to BexVm for constructing StackTrace instances from Vec<ErrorLocation> (used by Phase 5 catch binding)
Add optional second binding parameter to catch clauses that provides
a structured StackTrace object. The syntax `catch (e, st) { ... }`
binds the caught exception to `e` and the stack trace to `st`
(typed as baml.errors.StackTrace). Without the second parameter,
no stack trace is allocated — zero overhead for existing code.
Changes span every compiler layer:
- Parser: parse optional comma + second identifier after catch pattern
- SyntaxKind: add CATCH_STACK_TRACE_BINDING node type
- CST wrapper: add stack_trace_binding() accessor to CatchClause
- AST: add stack_trace_binding: Option<PatId> to CatchClause
- CST-to-AST lowering: extract second binding from CATCH_STACK_TRACE_BINDING
- HIR: register second binding in CatchClause scope
- TIR: type second binding as baml.errors.StackTrace via cross-package
lookup, insert into locals for name resolution
- MIR: declare stack_trace_local, store in CatchRegion
- Emit: add stack_trace_slot to ExceptionTableEntry
- VM: construct and store StackTrace instance when handler has stack_trace_slot
Merging this PR will not alter performance
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
baml_language/crates/baml_compiler2_mir/src/lower.rs (1)
963-967:⚠️ Potential issue | 🟡 MinorAlso seed lambda
MirFunction::span.This fixes top-level function metadata, but
lower_lambda()still creates a freshMirBuilderwithout callingset_span, so closureMirFunctions remain span-less. Any traceback/debug path that falls back to callable-level metadata can still lose source location for closure frames.Suggested follow-up
// in lower_lambda(), right after constructing the fresh builder self.builder = MirBuilder::new(Name::new(&lambda_name), arity); if let Some(span) = self.span_for_expr(expr_id) { self.builder.set_span(span); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_mir/src/lower.rs` around lines 963 - 967, lower_lambda() constructs a fresh MirBuilder for closures (via MirBuilder::new / self.builder) but never seeds its span, leaving MirFunction::span empty for lambdas; after creating the new builder in lower_lambda(), call self.span_for_expr(expr_id) (or otherwise obtain the closure expression span) and pass it to self.builder.set_span(...) so the newly-built MirFunction has its span populated for closure frames.
🧹 Nitpick comments (5)
baml_language/crates/baml_compiler_syntax/src/ast.rs (1)
2791-2796: Please confirm unit coverage forstack_trace_binding().Given this is new AST API surface, add/confirm a focused unit test for both
catch (e)andcatch (e, st)parsing paths.As per coding guidelines "Prefer writing Rust unit tests over integration tests where possible".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler_syntax/src/ast.rs` around lines 2791 - 2796, Add focused Rust unit tests to cover the new AST API stack_trace_binding(): write one test that parses a catch clause "catch (e)" and asserts stack_trace_binding() returns None, and another that parses "catch (e, st)" and asserts stack_trace_binding() returns Some node whose kind() equals SyntaxKind::CATCH_STACK_TRACE_BINDING; place these as unit tests in the same crate (near ast.rs tests) and use existing parser/helper utilities used elsewhere in the crate to obtain the catch clause SyntaxNode and call stack_trace_binding() for verification.baml_language/crates/baml_compiler_parser/src/parser.rs (1)
3119-3125: Add a parser unit test forcatch (e, st)grammar.This new branch introduces
CATCH_STACK_TRACE_BINDING; please add a parser-level unit test (including one malformed case likecatch (e,)) to lock CST shape and recovery behavior.As per coding guidelines "Prefer writing Rust unit tests over integration tests where possible".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler_parser/src/parser.rs` around lines 3119 - 3125, Add parser-level Rust unit tests that assert the CST shape and recovery for the new CATCH_STACK_TRACE_BINDING branch: write one positive test feeding "catch (e, st)" and asserting that the CST includes a CATCH_STACK_TRACE_BINDING node created by p.with_node (and that the second binding is a Word token), and one malformed test feeding "catch (e,)" that verifies recovery produces a CATCH_STACK_TRACE_BINDING node (or an expected error node) rather than panicking; place these tests alongside the parser unit tests in the same crate and use the existing parser test helpers/assertions to validate node kinds and token positions.baml_language/crates/bex_vm/src/package_baml/stack_trace.rs (1)
12-14: Consider defensive error handling instead ofexpect().The
expect("StackTrace: expected Instance")will panic if the value is not an instance. While this should never happen if the type system is working correctly, a more defensive approach could return an error message string instead of panicking.💡 Defensive alternative
- let instance = vm - .as_instance(stacktrace) - .expect("StackTrace: expected Instance"); + let Some(instance) = vm.as_instance(stacktrace) else { + return "<invalid StackTrace>".to_string(); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/bex_vm/src/package_baml/stack_trace.rs` around lines 12 - 14, Replace the panic-causing expect on vm.as_instance(stacktrace) with defensive error handling: call vm.as_instance(stacktrace) and if it returns Some(instance) continue, otherwise return or propagate an Err(String) (e.g., Err("StackTrace: expected Instance".into())) instead of panicking; update the surrounding function signature to return a Result or propagate the error through the existing error type so callers of the stack trace code (references: vm, as_instance, stacktrace, StackTrace) receive a proper error instead of aborting.baml_language/crates/baml_tests/tests/exceptions.rs (1)
3444-3464: Panic stack trace test is assertion-based rather than snapshot-based.Unlike the other stack trace tests that use
insta::assert_snapshot!, this test uses loose assertions (s.contains("Traceback")ands.contains("divider")). Consider using a snapshot assertion for consistency and to catch formatting regressions.💡 Suggestion: Use snapshot assertion for consistency
let result = output.result.unwrap(); - let s = format!("{result:?}"); - assert!(s.contains("Traceback"), "expected traceback, got: {s}"); - assert!(s.contains("divider"), "expected divider frame, got: {s}"); + insta::assert_snapshot!(format!("{result:?}"), `@r`#"String("Traceback (most recent call last):\n File \"test.baml\", line 7, in user.main\n File \"test.baml\", line 3, in user.divider")"#);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_tests/tests/exceptions.rs` around lines 3444 - 3464, The test uses loose assertions (s.contains("Traceback") / s.contains("divider")) instead of a snapshot; change the assertions to use insta::assert_snapshot! so the panic stack trace is snapshot-tested like the others: capture the stack output string (variable s) and call insta::assert_snapshot!(s) (or assert_snapshot!(s, @"...") if an inline expected is needed), replacing the contains checks; update the test that currently references risky and main and the output.bytecode snapshot block to include this new snapshot assertion to ensure formatting/regressions are caught.baml_language/crates/bex_vm/src/vm.rs (1)
1159-1176: Extract trace collection into one helper.Line 1162 starts a second inlined
Vec<ErrorLocation>builder that already diverges fromstack_trace()above on error handling (filter_map(...ok()?)here vsunwrap_or_default()there). That makes it easy for caughtcatch (e, st)traces and the publicstack_trace()API to drift apart on the next traceback change. A shared helper would keep file/line/function resolution consistent in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/bex_vm/src/vm.rs` around lines 1159 - 1176, Extract the inline Vec<ErrorLocation> builder into a single helper method (e.g., collect_stack_trace(&self) -> Vec<ErrorLocation>) and have both the inlined unwind code and the existing stack_trace() call that helper so resolution of function/file/line is centralized; implement the helper using the same error-handling semantics currently used by stack_trace() (preserve unwrap_or_default() behavior rather than filter_map(...ok()?)) and use the existing symbols (self.frames, self.get_object, ErrorLocation, func.bytecode.source_line_for_pc, func.name, func.source_file, func.span, stack_trace()) so both call sites produce identical trace output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 1737-1740: The code only handles Pattern::Binding when inserting
locals for the stack-trace binding; add handling for Pattern::TypedBinding as
well so declarations like catch (e, st: StackTrace) register the name for
resolution. Locate the match/if around body.patterns[st_binding] that currently
checks `if let baml_compiler2_ast::Pattern::Binding(name) =
&body.patterns[st_binding] { self.locals.insert(name.clone(), st_ty); }` and
extend it to also match `baml_compiler2_ast::Pattern::TypedBinding(name, _)` (or
otherwise extract the identifier from TypedBinding) and insert the same `st_ty`
into `self.locals`, mirroring the existing Binding behavior used for error
binding handling.
---
Duplicate comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 963-967: lower_lambda() constructs a fresh MirBuilder for closures
(via MirBuilder::new / self.builder) but never seeds its span, leaving
MirFunction::span empty for lambdas; after creating the new builder in
lower_lambda(), call self.span_for_expr(expr_id) (or otherwise obtain the
closure expression span) and pass it to self.builder.set_span(...) so the
newly-built MirFunction has its span populated for closure frames.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 3119-3125: Add parser-level Rust unit tests that assert the CST
shape and recovery for the new CATCH_STACK_TRACE_BINDING branch: write one
positive test feeding "catch (e, st)" and asserting that the CST includes a
CATCH_STACK_TRACE_BINDING node created by p.with_node (and that the second
binding is a Word token), and one malformed test feeding "catch (e,)" that
verifies recovery produces a CATCH_STACK_TRACE_BINDING node (or an expected
error node) rather than panicking; place these tests alongside the parser unit
tests in the same crate and use the existing parser test helpers/assertions to
validate node kinds and token positions.
In `@baml_language/crates/baml_compiler_syntax/src/ast.rs`:
- Around line 2791-2796: Add focused Rust unit tests to cover the new AST API
stack_trace_binding(): write one test that parses a catch clause "catch (e)" and
asserts stack_trace_binding() returns None, and another that parses "catch (e,
st)" and asserts stack_trace_binding() returns Some node whose kind() equals
SyntaxKind::CATCH_STACK_TRACE_BINDING; place these as unit tests in the same
crate (near ast.rs tests) and use existing parser/helper utilities used
elsewhere in the crate to obtain the catch clause SyntaxNode and call
stack_trace_binding() for verification.
In `@baml_language/crates/baml_tests/tests/exceptions.rs`:
- Around line 3444-3464: The test uses loose assertions (s.contains("Traceback")
/ s.contains("divider")) instead of a snapshot; change the assertions to use
insta::assert_snapshot! so the panic stack trace is snapshot-tested like the
others: capture the stack output string (variable s) and call
insta::assert_snapshot!(s) (or assert_snapshot!(s, @"...") if an inline expected
is needed), replacing the contains checks; update the test that currently
references risky and main and the output.bytecode snapshot block to include this
new snapshot assertion to ensure formatting/regressions are caught.
In `@baml_language/crates/bex_vm/src/package_baml/stack_trace.rs`:
- Around line 12-14: Replace the panic-causing expect on
vm.as_instance(stacktrace) with defensive error handling: call
vm.as_instance(stacktrace) and if it returns Some(instance) continue, otherwise
return or propagate an Err(String) (e.g., Err("StackTrace: expected
Instance".into())) instead of panicking; update the surrounding function
signature to return a Result or propagate the error through the existing error
type so callers of the stack trace code (references: vm, as_instance,
stacktrace, StackTrace) receive a proper error instead of aborting.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 1159-1176: Extract the inline Vec<ErrorLocation> builder into a
single helper method (e.g., collect_stack_trace(&self) -> Vec<ErrorLocation>)
and have both the inlined unwind code and the existing stack_trace() call that
helper so resolution of function/file/line is centralized; implement the helper
using the same error-handling semantics currently used by stack_trace()
(preserve unwrap_or_default() behavior rather than filter_map(...ok()?)) and use
the existing symbols (self.frames, self.get_object, ErrorLocation,
func.bytecode.source_line_for_pc, func.name, func.source_file, func.span,
stack_trace()) so both call sites produce identical trace output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7cbfb8c6-3570-4d1b-9041-09b3041cc4e7
⛔ Files ignored due to path filters (2)
baml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snap
📒 Files selected for processing (24)
baml_language/crates/baml_builtins2/baml_std/baml/ns_errors/stack_trace.bamlbaml_language/crates/baml_builtins2/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_mir/src/ir.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_tests/tests/.env.rs.pending-snapbaml_language/crates/baml_tests/tests/.exceptions.rs.pending-snapbaml_language/crates/baml_tests/tests/exceptions.rsbaml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/stack_trace.rsbaml_language/crates/bex_vm/src/types.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm_types/src/bytecode.rsbaml_language/crates/bex_vm_types/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- baml_language/crates/bex_vm_types/src/types.rs
- baml_language/crates/bex_vm/src/package_baml/mod.rs
- baml_language/crates/bex_vm/src/types.rs
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/crates/baml_compiler2_emit/src/emit.rs
…snapshots Delete .pending-snap files and properly accept inline snapshots via cargo insta. Update stack trace test assertions to pretty-print the actual string instead of using Debug formatting.
… formatting Remove the dead StackTrace struct and vm.stack_trace() method from bex_vm. Extract a shared format_traceback() function in errors.rs that both bex_engine::format_unhandled_throw and the BAML StackTrace.to_string() impl now delegate to.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
baml_language/crates/bex_vm/src/vm.rs (1)
1048-1049: Stale dead-code annotation/comment should be removed.
alloc_stack_traceis now actively used (Line 1185), so#[allow(dead_code)]and the “Phase 5” note are outdated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/bex_vm/src/vm.rs` around lines 1048 - 1049, Remove the stale dead-code suppression and outdated comment from the alloc_stack_trace function: delete the #[allow(dead_code)] attribute and the “Phase 5 when catch (e, st) is wired up.” note, since alloc_stack_trace is actively used (see usage around alloc_stack_trace call sites); just leave the pub(crate) fn alloc_stack_trace(&mut self, trace: &[ErrorLocation]) -> Value signature and body unchanged.baml_language/crates/bex_vm/src/errors.rs (1)
171-191: Add unit tests for traceback formatting edge cases.Please add crate-local unit tests for (1) empty iterator output and (2) multi-frame ordering/format stability.
As per coding guidelines "
**/*.rs: Prefer writing Rust unit tests over integration tests where possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/bex_vm/src/errors.rs` around lines 171 - 191, Add crate-local unit tests in the same file to cover format_traceback: create a #[cfg(test)] mod tests with two tests—one that calls format_traceback with an empty iterator and asserts it returns an empty String, and another that supplies a multi-frame iterator (e.g., vec![("test.baml", 3, "user.inner"), ("test.baml", 7, "user.main")].into_iter()) and asserts the exact Python-style multiline output and ordering (starts with "Traceback (most recent call last):" then each " File \"...\", line ..., in ..." line in the given order) using assert_eq! to ensure format stability for format_traceback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@baml_language/crates/bex_vm/src/errors.rs`:
- Around line 171-191: Add crate-local unit tests in the same file to cover
format_traceback: create a #[cfg(test)] mod tests with two tests—one that calls
format_traceback with an empty iterator and asserts it returns an empty String,
and another that supplies a multi-frame iterator (e.g., vec![("test.baml", 3,
"user.inner"), ("test.baml", 7, "user.main")].into_iter()) and asserts the exact
Python-style multiline output and ordering (starts with "Traceback (most recent
call last):" then each " File \"...\", line ..., in ..." line in the given
order) using assert_eq! to ensure format stability for format_traceback.
In `@baml_language/crates/bex_vm/src/vm.rs`:
- Around line 1048-1049: Remove the stale dead-code suppression and outdated
comment from the alloc_stack_trace function: delete the #[allow(dead_code)]
attribute and the “Phase 5 when catch (e, st) is wired up.” note, since
alloc_stack_trace is actively used (see usage around alloc_stack_trace call
sites); just leave the pub(crate) fn alloc_stack_trace(&mut self, trace:
&[ErrorLocation]) -> Value signature and body unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a9b2798-0274-49d7-a531-0dfd4236e2c2
📒 Files selected for processing (5)
baml_language/crates/bex_engine/src/lib.rsbaml_language/crates/bex_vm/src/errors.rsbaml_language/crates/bex_vm/src/lib.rsbaml_language/crates/bex_vm/src/package_baml/stack_trace.rsbaml_language/crates/bex_vm/src/vm.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/crates/bex_vm/src/lib.rs
- baml_language/crates/bex_engine/src/lib.rs
- baml_language/crates/bex_vm/src/package_baml/stack_trace.rs
…ndings - Rename ErrorLocation to StackFrame to match the BAML class name - Remove stale #[allow(dead_code)] on alloc_stack_trace (now actively used) - Populate source_file for compiled lambdas so closure frames show filenames in tracebacks - Handle TypedBinding pattern for stack trace local in TIR builder so catch (e, st: StackTrace) works
…stead of raw indices The $init function was built by pushing instructions directly into a Vec, bypassing the Emitter that populates the parallel meta vector. This caused store_global/call to display as ?141 instead of the fully-qualified binding name. Now we build a parallel init_meta vec with Callable, Global, and Const metadata for each instruction.
Windows CI checks out files with CRLF, which causes include_str! to embed CRLF content for builtin .baml files. This shifts byte offsets in build_line_starts, producing different source line numbers in bytecode snapshots. Force LF so all platforms get identical content.
antoniosarosi
changed the title
This was referenced Apr 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Artifacts | Task
What problems was I solving
Unhandled throws reaching Python/TypeScript/WASM included no call stack context — just a bare "uncaught throw: ..." message. Debugging required guesswork to trace which function threw and through which call chain. Additionally,
BexVm::stack_trace()silently returned empty traces whenever closure frames were on the stack (theas_function()call failed onObject::Closure), and bytecode line tables were always empty because MIR statements carried no source spans.What user-facing changes did I ship
Displayimpl — no bridge changes neededHow I implemented it
Phase 1: VM-layer plumbing
bex_vm_types/src/types.rs— Addedsource_file: Stringfield toFunctionstruct, populated at emit time with the filesystem pathbex_vm/src/types.rs— Addedas_callable()toObjectTraitthat handles bothObject::FunctionandObject::Closure, fixing the silent-empty-trace bugbex_vm/src/errors.rs— Addedfile_path: StringtoErrorLocation, addedtrace: Vec<ErrorLocation>toVmError::UnhandledThrow, updatedStackTrace::Displayto use file pathsbex_vm/src/vm.rs— Updatedstack_trace()andReturnhandler to useas_callable(). Added trace capture at the top oftry_unwind_exception()before frame unwinding destroys the call stackbex_vm/src/package_baml/mod.rs— Updatedattach_builtinsFunction copy site to includesource_filebaml_compiler2_emit/src/lib.rs— Setsource_filefromfile.path(db)at all Function construction sites (bytecode, builtin IO, builtin VM, synthesized)baml_compiler2_emit/src/emit.rs— Addedsource_file: String::new()tocompile_mir_function's Function constructionPhase 1.5: Line table population
baml_compiler2_mir/src/lower.rs— Addedspan_for_expr()andspan_for_stmt()helpers that buildSpanfrom the AST source map. Added save/restore ofbuilder.current_source_spanat top/bottom oflower_expr()andlower_stmt()— all ~68assign()call sites automatically inherit spans. Calledbuilder.set_span()inlower_function_body()to populateMirFunction::spanbaml_compiler2_emit/src/emit.rs— Updatedcompile_mir_functionto acceptmir_span: Option<Span>and apply it toFunction::spanbaml_compiler2_emit/src/lib.rs— Passedmir.span/lambda.span/Noneat all 3 call sitesPhase 2: Engine-layer propagation
bex_engine/src/lib.rs— Addedtrace: Vec<ErrorLocation>toEngineError::UnhandledThrow, addedformat_unhandled_throwhelper for Python-style Display formatting, forwarded trace fromVmErrormatch armbex_vm/src/lib.rs— Addedpub use errors::ErrorLocationre-exportTests
baml_tests/tests/errors.rs— Un-ignorederror_stack_trace, implemented with 4-frame call chain assertions, file_path and error_line > 0 verificationbaml_tests/tests/exceptions.rs— Updated 4UnhandledThrowmatch sites to use{ value, .. }. Addedexception_stack_trace_through_closuresandexception_stack_trace_on_panictestsDeviations from the plan
Implemented as planned
as_callable()dual-dispatch pattern, trace capture before unwinding,format_unhandled_throwhelper, MIR span threading viacurrent_source_spansave/restore — all match the planDeviations/surprises
source_line_for_pcreturnsusizedirectly (notOption<usize>) — plan snippets showed.unwrap_or(0)but the actual API doesn't need it$init_let_*helpers also getsource_fileset (plan didn't mention this site)attach_builtinsinpackage_baml/mod.rsneeded updating for the newsource_filefield (plan's "search all construction sites" didn't enumerate this one)Additions not in plan
PartialEqderived onErrorLocation(required byEngineError's derive chain)#[allow(unsafe_code)]onObjectTraitimpl blockItems planned but not implemented
How to verify it
Setup
git fetch git checkout antonio/add-stack-trace-api-to-error-handling cd baml_languageAutomated Tests
Description for the changelog
Unhandled throws now include Python-style stack traces with file paths and line numbers, automatically surfaced through all FFI bridges.
Summary by CodeRabbit
New Features
Bug Fixes
Tests