Support capturing Error backtraces and debug.traceback

[rib]

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]

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

[Aeledfyr]

I haven't had a chance to look through this fully, but it looks really good! Thank you!

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. There are a few discussions on the piccolo discord channel on it, such as this one, but the general gist is that making it compatible with base Lua is hard because Lua generally converts errors to strings and makes passing through metadata difficult.

(I'll also see if I can fix the CI issue, should be doable on my end.)

[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.

[Aeledfyr]

With this PR, RuntimeError and LuaError store the backtrace internally, but ExternLuaError doesn't, and puts it in the ExternError struct. It seems like these would be easier to manage if they were all the same? In addition, that would allow preserving the conversion from LuaError to ExternLuaError.

Edit: Also, StashedError doesn't preserve the backtrace, which may make errors lossy in async_sequence-based callbacks.

Also, the current approach in piccolo for error formatting is using {:#} on RuntimeError in the Display impl for Error/ExternError, which the RuntimeError passes to anyhow to generate a single-line error chain for the errors. ("runtime error: operator error: could not add values of type table and number").

This PR currently replaces that with manual chain printing (#[error("operator error: {0}")]) in VMError, since the alternate mode was reinterpreted to mean printing the backtrace, but it may be better to fix that by always enabling alt mode when printing the anyhow error (in impl Display for RuntimeError, and in pretty_print_error_with_backtrace).

(I also don't know the best way of exposing the enabling of backtrace printing to the user -- there are four general options for formatting ({}, {:#}, {:?} and {:#?}), and anyhow treats all of them differently, but I'm not sure what the best approach for piccolo would be.)

[Aeledfyr]

This may be easier to use if it returns impl fmt::Display rather than taking a Write implementation; for an example see

piccolo/src/value.rs

Lines 55 to 59 in ce709eb

	pub fn display(self) -> impl fmt::Display + 'gc {
	struct ValueDisplay<'gc>(Value<'gc>);
	impl<'gc> fmt::Display for ValueDisplay<'gc> {
	fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> std::fmt::Result {

(Actually, pretty_print_error_with_backtrace could itself return impl fmt::Display to reduce code duplication from monomorphization.)

[Aeledfyr]

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]

This could be simplified to

for frame in frames.iter().rev() {
    // ...
}

(i is unused elsewhere, but if you needed it, it would be for (i, frame) in frames.iter().enumerate().rev() { ...)

[Aeledfyr]

LuaError::from is unnecessary here and in the definition of error

[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.