Add stringified backtrace to WrappedError passed back to Lua

[horazont]

This makes the behaviour consistent with how Lua behaves when a
normal string is used as error instead of a userdata.

When a string is used, the Lua runtime will add a complete
stacktrace to the error output; when a userdata is used, it will
not do that, but only display the tostring(ud) at the time the
error is printed.

This makes debugging things much harder (especially when writing
modules), because errors emitted by the Rust code (such as type
conversion errors) are not attribute to a code line.

However, this implementation has two important caveats:

• The runtime complexity of walking the stack and transforming it
  into a nice human-readable string; this complexity is to be
  expected to also exist in case of Lua doing that internally,
  modulo extra cost for utf8 checks inside Rust.

• When the WrappedError is transported back into and out of Rust,
  the stacktrace is lost and regenerated when passed back into
  Lua.

  This means that the error cause can be "shadowed" by passing
  through Rust.

  Handling this would require a more invasive change which allows
  chaining Error instances in a way which allows carrying the
  backtrace without changing the Error enum to contain an optional
  stacktrace in each variant; I’ll leave this to future work.

Fixes #44.

[khvzak]

Lua runs GC only during lua_load / lua_pcall calls, so it's safe to pop value in this case.

[khvzak]

Thanks for PR!

Overall it looks good.

As you mentioned in the commit message, attached stacktrace will be lost during WrappedError -> Error conversion and it's quite hard to preserve it. However, I'm sure preserving stacktrace in the CallbackError variant would cover many (if not all) cases. But with more tricky logic to format a correct error message.

I ran a few checks and seems this solution unfortunately is not comprehensive and leaves a few edge cases where stack trace would be incomplete or incorrectly formatted:

Code example:

fn rust_error(lua: &Lua, _: ()) -> LuaResult<()> {
    lua.globals().get::<_, LuaFunction>("rust_error2")?.call(())
}

fn rust_error2(lua: &Lua, _: ()) -> LuaResult<()> {
    lua.globals().get::<_, LuaFunction>("rust_error3")?.call(())
}

fn rust_error3(_: &Lua, _: ()) -> LuaResult<()> {
    Err(LuaError::RuntimeError("rust error".into()))
}

fn lua_error(lua: &Lua, _: ()) -> LuaResult<()> {
    lua.globals().get::<_, LuaFunction>("lua_error2")?.call(())
}

fn lua_error2(lua: &Lua, _: ()) -> LuaResult<()> {
    lua.globals().get::<_, LuaFunction>("lua_error3")?.call(())
}

fn lua_error3(lua: &Lua, _: ()) -> LuaResult<()> {
    lua.load("error(\"lua error\")").exec()
}

#[mlua::lua_module]
fn my_module(lua: &Lua) -> LuaResult<bool> {
    let globals = lua.globals();
    for (k, v) in vec![
        ("rust_error", lua.create_function(rust_error)?),
        ("rust_error2", lua.create_function(rust_error2)?),
        ("rust_error3", lua.create_function(rust_error3)?),
        ("lua_error", lua.create_function(lua_error)?),
        ("lua_error2", lua.create_function(lua_error2)?),
        ("lua_error3", lua.create_function(lua_error3)?),
    ] {
        globals.set(k, v)?;
    }
    Ok(true)
}

And test cases:

--

$ lua -l my_module -e 'rust_error()'
lua: callback error: stack traceback:
        [C]: in ?
        [C]: in function 'rust_error2'
        [C]: in function 'rust_error'
        (command line):1: in main chunk
        [C]: in ?
stack traceback:
        [C]: in function "rust_error"
        (command line):1: in main chunk
        [C]: in ?

No trace that the error was triggered in the rust_error3 function and double stacktrace.
No cause rust error.

--

$ lua -l my_module -e 'lua_error()'
lua: callback error: stack traceback:
        [C]: in ?
        [C]: in function 'lua_error2'
        [C]: in function 'lua_error'
        (command line):1: in main chunk
        [C]: in ?
stack traceback:
        [C]: in function "lua_error"
        (command line):1: in main chunk
        [C]: in ?

The error cause lua error is missing. Double stacktrace.

--

$ lua -l my_module -e 'lua_error3()'
lua: runtime error: [string "?"]:1: lua error
stack traceback:
        [C]: in ?
        [C]: in function 'error'
        [string "?"]:1: in main chunk
        [C]: in function 'lua_error3'
        (command line):1: in main chunk
        [C]: in ?
stack traceback:
        [C]: in function "lua_error3"
        (command line):1: in main chunk
        [C]: in ?

double stacktrace again

Also a few comments about get_traceback and default stacktrace size (128 here):

• I believe could be more reasonable having a full get_traceback implementation with all that magic to get global function names (pushglobalfuncname, etc). Otherwise there a less advantages over the native luaL_traceback.
• Would be nice to have configurable stacktrace limit, using LuaOptions in a standalone mode, or via derive args in mlua_derive.

[horazont]

Thanks for PR!

Overall it looks good.

Thank you :)

As you mentioned in the commit message, attached stacktrace will be lost during WrappedError -> Error conversion and it's quite hard to preserve it. However, I'm sure preserving stacktrace in the CallbackError variant would cover many (if not all) cases. But with more tricky logic to format a correct error message.

Yes, I agree.

I ran a few checks and seems this solution unfortunately is not comprehensive and leaves a few edge cases where stack trace would be incomplete or incorrectly formatted:

Did you run the checks with an implementation where CallbackError is used to preserve the stacktrace, or did you manage to produce those traces with the current implementation?

Also a few comments about get_traceback and default stacktrace size (128 here):

• I believe could be more reasonable having a full get_traceback implementation with all that magic to get global function names (pushglobalfuncname, etc). Otherwise there a less advantages over the native luaL_traceback.

• Would be nice to have configurable stacktrace limit, using LuaOptions in a standalone mode, or via derive args in mlua_derive.

I agree on both points. How would you prefer to proceed?

• Update this PR to use luaL_traceback
• Update this PR to support pushglobalfuncname things
• Update this PR to support a configurable stacktrace limit
• Implement the CallbackError chaining
• Leave this PR as-is and leave the other suggestions for "future work"

[khvzak]

I made some changes and released a new version that addresses a missing tracebacks in a module mode.
Probably this PR is a bit outdated although if you want to contribute to the project I would happy to see Rust-based implementation of luaL_traceback with a String buffer and compatible with Lua 5.1 (jit) - 5.4

I already have this function implemented https://github.com/khvzak/mlua/blob/v0.6.0-beta.2/src/ffi/compat53.rs#L676 that I believe would require minimal modifications, to not to trigger 'm' errors.

[horazont]

@khvzak I rebuilt my project with mlua master, and it looks very nice, thanks!