piccolo icon indicating copy to clipboard operation
piccolo copied to clipboard

Support capturing Error backtraces and `debug.traceback`

Open rib opened this issue 6 months ago • 4 comments

This adds optional backtraces to Error::Runtime and Error::Lua which are automatically captured when step() sees an error.

When '#' / alternative formatting is specified, the Display implementation for Errors will pretty-print any backtrace like:

Error: lua error: error: test error from rust
stack traceback:
  test.lua:4 in <function 'error_callback' at line 1>
    arguments:
      err_msg: test error from rust
  test.lua:9 in <function 'baz' at line 7>
    arguments:
      a: test
      b:  error from rust
  test.lua:13 in <function 'bar' at line 12>
    arguments:
      x: test
      ...:  error from rust
  test.lua:19 in <function 'foo' at line 16>
    arguments:
      arg1: test
  test.lua:22 in <chunk>

There is also a pretty_print() method that can be given a source map (that maps chunk names to source code) that will include a line of source code for each frame, like:

Error: lua error: error: test error from rust
stack traceback:
  test.lua:4 in <function 'error_callback' at line 1>: `error(err_msg)`
    arguments:
      err_msg: test error from rust
  test.lua:9 in <function 'baz' at line 7>: `error_callback(a .. b)`
    arguments:
      a: test
      b:  error from rust
  test.lua:13 in <function 'bar' at line 12>: `baz(x, ...)`
    arguments:
      x: test
      ...:  error from rust
  test.lua:19 in <function 'foo' at line 16>: `bar(arg1, " error from rust")`
    arguments:
      arg1: test
  test.lua:22 in <chunk>: `foo("test")`

The debug.traceback implementation doesn't try and conform to the same traceback format as PUC-Lua, or any other Lua implementation, but should honour the optional thread, msg or level arguments.

rib avatar Jul 08 '25 23:07 rib

It looks like the Circle CI / Miri failures also show up on master and aren't introduced by this PR

rib avatar Jul 08 '25 23:07 rib

The only design question here (meaning I'll need to wait for approval from kyren for merging) is whether to automatically attach backtraces to errors, and how that influences Lua compatibility.

I guess from Lua code the change should be transparent, and if Lua code wants to see a backtrace it needs to explicitly call debug.traceback. Might need to double check that the backtrace doesn't end up in the strings that Lua gets back for errors, but that should be avoidable so long as errors are converted to strings without '#'/alternate formatting.

At some point I did start trying to add some kind of ref-count tracking of whether there was a PCall sequence on the stack that could catch an error so that the implementation could skip capturing a backtrace if it looks like the error is going to be caught but in the end that felt overly-fiddly at this stage.

One thought was that potentially there could be global flag, akin to RUST_BACKTRACE=1 for controlling when to automatically capture backtraces, but also figured that something like that could be added later if it would be useful.

rib avatar Jul 10 '25 13:07 rib

I've fixed the CI issue and an error downcasting bug (#120, fixed in #123) on master; I don't think there should be any conflicts, but it's worth checking. It's also worth checking if the error changes from this PR would re-introduce #120, since the error downcasting is tied pretty closely to the error format.

Aeledfyr avatar Jul 10 '25 21:07 Aeledfyr

Okay, I've checked with kyren;

  • Auto-capturing backtraces is fine, and should be the default; we don't really need config for the initial version
  • The Vec<BacktraceFrame> should have a wrapper struct; then pretty_print_error_with_backtrace would become a method on that struct
  • It might be slightly cleaner to store the backtraces in the Error rather than in each variant, but that means turning it into struct Error { kind: ErrorKind, backtrace: Option<Vec<BacktraceFrame>> } for each of Error, ExternError, and StashedError. I don't know if this would actually be simpler, or worth the effort.

In the longer term, an executor will likely have an error hook function called whenever an error is initially thrown (when a new Frame::Error is pushed when not currently handling an error); then that hook could capture the backtrace and attach it to the error without having to have a backtrace field in LuaError/RuntimeError, by using a custom rust error type. This would allow the user to control how the backtraces are captured, as well as if/how they're exposed to Lua.

Aeledfyr avatar Jul 11 '25 05:07 Aeledfyr