grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Script errors reach the error handler bare, as 1.12.1 hands them (#865)

Browse Source 
Script errors reached the Lua error handler as mlua's display text: a `runtime error: ` word and a `stack traceback:` block, which overflow the stock `ScriptErrors` dialog. And a raise in a `CreateFrame` template's OnLoad was logged as a warning and never reached the handler at all.

In the reference, handlers run under `lua_pcall` with the error function `0x703b40`, which calls the `seterrorhandler` function with the bare message. `CreateFrame` builds its template through the XML instantiator `0x6ee280`, whose OnLoad runs under that same function (`0x704d50`).

Now every caught error records two texts: the decorated one for benilla's log and the bare message for the handler. A load report pairs the two per row, and `CreateFrame` records its template's errors as script errors.

Tests: the handler hears bare messages from handlers, and a template OnLoad raise reaches it.

Fixes #697
Fixes #859

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Sam Sung
2026-10-11 19:12:21 +07:00
committed by GitHub
co-authored by Claude Opus 5.5
parent f356e90fe6
commit c9ddd80342
30 changed files with 308 additions and 115 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/ui_script/addons.rs
+75 -5
View File
@@ -158,11 +158,11 @@ impl Addon {
// order, as `AddOn_Load 0x51f240` hands every listed file to `0x6edb90`.
match script.run_chunk_named(&bytes, &self.chunk_name(file, &path)) {
Ok(()) => info!("ui_script: {}/{file} ran", self.name),
Err(e) => {
let e = format!("{}/{file}: {e}", self.name);
Err(err) => {
let e = format!("{}/{file}: {err}", self.name);
error!("ui_script: {e}");
// A script error: it reaches the player through the Lua error handler.
script.report_script_error(&e);
script.report_script_error(&e, &benilla_ui::script::lua_message(&err));
failures.push(e);
}
}
@@ -210,11 +210,11 @@ impl Addon {
script.report_load_failure(&e);
failures.push(e);
}
for e in &report.errors {
for (e, heard) in report.raised() {
error!("ui_script({}/{file}): {e}", self.name);
// A script error, as in the Lua arm; `_ERRORMESSAGE`'s IsVisible guard shows only
// a burst's first, as in the reference.
script.report_script_error(&format!("{}/{file}: {e}", self.name));
script.report_script_error(&format!("{}/{file}: {e}", self.name), heard);
failures.push(format!("{}/{file}: {e}", self.name));
}
info!(
@@ -2891,4 +2891,74 @@ mod tests {
);
let _ = std::fs::remove_dir_all(home.parent().unwrap());
}
/// The error handler hears what 1.12's `lua_pcall` error function hands it (`0x703b40`, from
/// `FS_DoBuffer 0x704ae0` for a file's top level and its syntax error, `0x704f10` for a
/// handler): the bare Lua message, with no frame, `OnLoad: `, addon or file word and no
/// traceback. The decorated text stays on the failures list and the host log.
#[test]
fn the_error_handler_hears_the_bare_message_of_every_load_failure() {
let tmp = std::env::temp_dir().join(format!("benilla-addon-bare-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&tmp);
let addon = dir_addon(
&tmp,
"Bare",
"## Interface: 11200\ntop.lua\nsyntax.lua\nframe.xml\n",
);
std::fs::write(tmp.join("Bare/top.lua"), "local t = nil\nt.x = 1\n").unwrap();
std::fs::write(tmp.join("Bare/syntax.lua"), "x = = 1\n").unwrap();
std::fs::write(
tmp.join("Bare/frame.xml"),
r#"<Ui><Frame name="BareFrame"><Scripts><OnLoad>this:NoSuchMethod()</OnLoad></Scripts></Frame>
<Script>local u = nil; u.y = 2</Script></Ui>"#,
)
.unwrap();
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
script
.run("Heard = {} seterrorhandler(function(m) table.insert(Heard, m) end)")
.unwrap();
let failures = addon.load(&script, &mut Status::default());
assert_eq!(failures.len(), 4, "{failures:#?}");
script.dispatch_script_errors_to_handler();
let heard: Vec<String> = script.eval("return Heard").unwrap();
assert_eq!(heard.len(), 4, "{heard:#?}");
for m in &heard {
assert!(
!m.contains("runtime error")
&& !m.contains("syntax error")
&& !m.contains("stack traceback")
&& !m.contains("OnLoad: ")
&& !m.contains("Bare/")
&& !m.contains('\n'),
"{m:?}"
);
}
assert!(
heard[0].ends_with("top.lua:2: attempt to index local `t' (a nil value)"),
"{:?}",
heard[0]
);
assert!(
heard[0].starts_with("Interface\\AddOns\\Bare\\top.lua"),
"{:?}",
heard[0]
);
assert!(
heard[1].contains("syntax.lua:1:") && heard[1].contains("unexpected symbol"),
"{:?}",
heard[1]
);
assert!(
heard[2].ends_with("attempt to call method `NoSuchMethod' (a nil value)"),
"{:?}",
heard[2]
);
assert!(
heard[3].ends_with("attempt to index local `u' (a nil value)"),
"{:?}",
heard[3]
);
let _ = std::fs::remove_dir_all(&tmp);
}
}
crates/benilla-ui/src/loader/backdrop.rs
+1 -1
View File
@@ -15,7 +15,7 @@ impl Loader<'_> {
let table = match self.build_backdrop_table(bd) {
Ok(t) => t,
Err(e) => {
self.report.errors.push(format!("{dbg}: <Backdrop>: {e}"));
self.report.fault(format!("{dbg}: <Backdrop>: {e}"));
return;
}
};
crates/benilla-ui/src/loader/geometry.rs
+1 -3
View File
@@ -146,9 +146,7 @@ impl Loader<'_> {
let region: Table = match wrapper.call_method("CreateTitleRegion", ()) {
Ok(r) => r,
Err(e) => {
self.report
.errors
.push(format!("{dbg}: CreateTitleRegion: {e}"));
self.report.fault(format!("{dbg}: CreateTitleRegion: {e}"));
return;
}
};
crates/benilla-ui/src/loader/mod.rs
+48 -23
View File
@@ -22,6 +22,33 @@ mod regions;
mod scripts;
mod widgets;
impl LoadReport {
/// A load fault that is no script raise: the handler hears the row's own text.
pub(super) fn fault(&mut self, text: String) {
self.handler_errors.push(text.clone());
self.errors.push(text);
}
/// A script raise: the row is `text`, the handler hears the bare Lua message of `e`.
pub(super) fn raise(&mut self, text: String, e: &mlua::Error) {
self.handler_errors.push(crate::script::lua_message(e));
self.errors.push(text);
}
/// Each of [`Self::errors`] with what the Lua error handler is called with for it.
pub fn raised(&self) -> impl Iterator<Item = (&str, &str)> {
debug_assert_eq!(
self.errors.len(),
self.handler_errors.len(),
"every error row is written through `fault` or `raise`"
);
self.errors
.iter()
.map(String::as_str)
.zip(self.handler_errors.iter().map(String::as_str))
}
}
/// The outcome of a [`load`]; nothing aborts a load, as the reference logs and continues.
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct LoadReport {
@@ -31,6 +58,11 @@ pub struct LoadReport {
/// Failures the reference raises on: a handler that fails to compile, a method call that
/// errors, a malformed included document.
pub errors: Vec<String>,
/// What the Lua error handler is called with for each of [`Self::errors`], row for row
/// ([`Self::raised`]): the bare Lua message of a raise, as `lua_pcall` leaves it for the
/// reference's error function (`0x703b40`), and the row's own text for a fault that is no
/// script raise.
handler_errors: Vec<String>,
/// `<Include file=>` and `<Script file=>` paths the provider lacks. The reference logs
/// `"Couldn't open %s"` (`0x6edaa0`, `0x846ff4`) or `"Error loading %s"` (`0x872e50`) and goes
/// on (`0x6ee00d`); kept apart from `warnings` so a load that resolved nothing is not clean.
@@ -101,15 +133,17 @@ pub fn load_into_under(
loader.report
}
/// Apply a template to an existing frame, `CreateFrame`'s fourth argument. Returns the warnings,
/// then the errors. The frame always survives: an unknown name leaves it bare, as in the
/// reference, and only virtual elements are registered. The frame's own name is the `$parent`
/// Apply a template to an existing frame, `CreateFrame`'s fourth argument. Returns the report:
/// its errors are script errors for the error handler, as the reference's `CreateFrame` builds the
/// template through the XML instantiator `0x6ee280`, whose OnLoad runs under the error function
/// (`0x704d50`). The frame always survives an error, and only virtual elements are registered; an
/// unknown name never gets here, since the binding raises first. The frame's own name is the `$parent`
/// base (`MyButton` makes `$parentText` `MyButtonText`). Runs inside the `CreateFrame` binding, so
/// it enters at [`Loader::decorate`]: [`Loader::materialize`] would recurse.
pub fn apply_template(lua: &mlua::Lua, wrapper: &Table, kind: &str, template: &str) -> Vec<String> {
pub fn apply_template(lua: &mlua::Lua, wrapper: &Table, kind: &str, template: &str) -> LoadReport {
let template = template.trim();
if template.is_empty() {
return Vec::new();
return LoadReport::default();
}
// A template never reads a file: `<Include>` and `<Script file=>` are top-level only.
let no_files = |_: &str| -> Option<Vec<u8>> { None };
@@ -179,9 +213,7 @@ pub fn apply_template(lua: &mlua::Lua, wrapper: &Table, kind: &str, template: &s
loader.decorate(&expanded, wrapper, &self_name, &parent_name, &dbg);
let mut out = loader.report.warnings;
out.extend(loader.report.errors);
out
loader.report
}
/// Join a FrameXML reference to its file's directory, lexically: `\` and `/` both separate, `..`
@@ -352,8 +384,7 @@ impl Loader<'_> {
Some(bytes) => {
if let Err(e) = self.run(crate::source::chunk(&bytes), &joined) {
self.report
.errors
.push(format!("<Script file=\"{path}\">: {e}"));
.raise(format!("<Script file=\"{path}\">: {e}"), &e);
}
}
None => {
@@ -377,7 +408,7 @@ impl Loader<'_> {
// `[string "Interface\FrameXML\Foo.xml:<Scripts>"]`, not a file path.
let name = format!("{}:<Scripts>", self.path.replace('/', "\\"));
if let Err(e) = self.run_named(&chunk, &name) {
self.report.errors.push(format!("inline <Script>: {e}"));
self.report.raise(format!("inline <Script>: {e}"), &e);
}
}
TopLevel::Font(el) => self.do_font(el),
@@ -418,8 +449,7 @@ impl Loader<'_> {
if status::runs_as_lua(&joined) {
if let Err(e) = self.run(crate::source::chunk(&bytes), &joined) {
self.report
.errors
.push(format!("<Include file=\"{path}\">: {e}"));
.raise(format!("<Include file=\"{path}\">: {e}"), &e);
}
return;
}
@@ -441,9 +471,7 @@ impl Loader<'_> {
Err(e) => {
let line = status::unparsed(&self.status_path(&joined));
self.report.status.report(status::FAILURE, line);
self.report
.errors
.push(format!("<Include file=\"{path}\">: {e}"))
self.report.fault(format!("<Include file=\"{path}\">: {e}"))
}
}
}
@@ -600,8 +628,7 @@ impl Loader<'_> {
Ok(f) => f,
Err(e) => {
self.report
.errors
.push(format!("CreateFrame global missing: {e}"));
.fault(format!("CreateFrame global missing: {e}"));
return None;
}
};
@@ -609,7 +636,7 @@ impl Loader<'_> {
match create.call((el.tag.clone(), resolved_name.clone(), parent.cloned())) {
Ok(w) => w,
Err(e) => {
self.report.errors.push(format!(
self.report.fault(format!(
"CreateFrame(<{}>{}): {e}",
el.tag,
resolved_name
@@ -699,7 +726,7 @@ impl Loader<'_> {
// and gives the ScrollFrame its scroll range. An empty one errors in the reference.
for sc in children_named(el, "ScrollChild") {
let Some(child) = sc.children.first() else {
self.report.errors.push(format!(
self.report.fault(format!(
"{dbg_name}: <ScrollChild> is empty — the reference errors here, and a \
ScrollFrame with no child has no scroll range at all"
));
@@ -710,9 +737,7 @@ impl Loader<'_> {
continue; // materialize already reported why
};
if let Err(e) = wrapper.call_method::<()>("SetScrollChild", child_wrapper) {
self.report
.errors
.push(format!("{dbg_name}: <ScrollChild>: {e}"));
self.report.fault(format!("{dbg_name}: <ScrollChild>: {e}"));
}
}
crates/benilla-ui/src/loader/regions.rs
+7 -15
View File
@@ -53,13 +53,13 @@ impl Loader<'_> {
match wrapper.call_method(method, (rname, Some(level.clone()))) {
Ok(w) => w,
Err(e) => {
self.report.errors.push(format!("{dbg}: {method}: {e}"));
self.report.fault(format!("{dbg}: {method}: {e}"));
continue;
}
};
if region.attr_bool("hidden") {
if let Err(e) = region_wrapper.call_method::<()>("Hide", ()) {
self.report.errors.push(format!("{dbg}: region Hide: {e}"));
self.report.fault(format!("{dbg}: region Hide: {e}"));
}
}
if let Some(a) = region
@@ -85,9 +85,7 @@ impl Loader<'_> {
if let Err(e) =
crate::script::implicit_creation_anchor_lua(self.lua, &region_wrapper)
{
self.report
.errors
.push(format!("{dbg}: implicit anchor: {e}"));
self.report.fault(format!("{dbg}: implicit anchor: {e}"));
}
}
}
@@ -141,8 +139,7 @@ impl Loader<'_> {
Ok(w) => w,
Err(e) => {
self.report
.errors
.push(format!("{dbg}: CreateFontString (special): {e}"));
.fault(format!("{dbg}: CreateFontString (special): {e}"));
continue;
}
},
@@ -152,9 +149,7 @@ impl Loader<'_> {
self.apply_region_visual(region, &region_wrapper, false, dbg);
// The implicit creation anchor, as in `apply_layers`; an EditBox's adopt replaces it.
if let Err(e) = crate::script::implicit_creation_anchor_lua(self.lua, &region_wrapper) {
self.report
.errors
.push(format!("{dbg}: implicit anchor: {e}"));
self.report.fault(format!("{dbg}: implicit anchor: {e}"));
}
// It is the EditBox's text region by slot, never found by a search, which could take a
// `<Layers>` string; the box's insets anchor it.
@@ -163,8 +158,7 @@ impl Loader<'_> {
crate::script::adopt_text_region(self.lua(), wrapper, &region_wrapper)
{
self.report
.errors
.push(format!("{dbg}: adopt special FontString: {e}"));
.fault(format!("{dbg}: adopt special FontString: {e}"));
}
}
}
@@ -383,9 +377,7 @@ impl Loader<'_> {
height,
outline,
) {
self.report
.errors
.push(format!("{dbg}: region font attrs: {e}"));
self.report.fault(format!("{dbg}: region font attrs: {e}"));
}
}
None => {}
crates/benilla-ui/src/loader/scripts.rs
+4 -7
View File
@@ -102,8 +102,7 @@ impl Loader<'_> {
Ok(f) => Some(f),
Err(e) => {
self.report
.errors
.push(format!("{dbg}: compiling <{name}>: {e}"));
.raise(format!("{dbg}: compiling <{name}>: {e}"), &e);
None
}
}
@@ -113,7 +112,7 @@ impl Loader<'_> {
/// (`0x704d50`); an error is recorded, never propagated.
pub(super) fn fire_onload(&mut self, wrapper: &Table, func: &Function, dbg: &str) {
if let Err(e) = self.invoke_handler(wrapper, func) {
self.report.errors.push(format!("{dbg}: OnLoad: {e}"));
self.report.raise(format!("{dbg}: OnLoad: {e}"), &e);
}
}
@@ -126,7 +125,7 @@ impl Loader<'_> {
dbg: &str,
) {
if let Err(e) = wrapper.call_method::<()>(method, args) {
self.report.errors.push(format!("{dbg}: {method}: {e}"));
self.report.fault(format!("{dbg}: {method}: {e}"));
}
}
@@ -139,9 +138,7 @@ impl Loader<'_> {
dbg: &str,
) {
if let Err(e) = region.call_method::<()>(method, args) {
self.report
.errors
.push(format!("{dbg}: region {method}: {e}"));
self.report.fault(format!("{dbg}: region {method}: {e}"));
}
}
}
crates/benilla-ui/src/loader/widgets.rs
+3 -7
View File
@@ -165,9 +165,7 @@ impl Loader<'_> {
self.call_region(&region, "ClearAllPoints", (), dbg);
self.apply_region_layout(bt, &region, self_name, dbg, font_attrs);
if let Err(e) = crate::script::implicit_creation_anchor_lua(self.lua, &region) {
self.report
.errors
.push(format!("{dbg}: implicit anchor: {e}"));
self.report.fault(format!("{dbg}: implicit anchor: {e}"));
}
// Published both ways, like a named state texture.
if let Some(rname) = bt_name {
@@ -232,8 +230,7 @@ impl Loader<'_> {
self.lua, wrapper, which, jh,
) {
self.report
.errors
.push(format!("{dbg}: <{}> justifyH: {e}", f.tag));
.fault(format!("{dbg}: <{}> justifyH: {e}", f.tag));
}
}
// Reported, not modelled: the reference clears the axis on a cross-axis
@@ -561,8 +558,7 @@ impl Loader<'_> {
outline,
) {
self.report
.errors
.push(format!("{dbg}: <SimpleHTML> {name} font attrs: {e}"));
.fault(format!("{dbg}: <SimpleHTML> {name} font attrs: {e}"));
}
}
None => {}
crates/benilla-ui/src/script/addon.rs
+21 -12
View File
@@ -525,7 +525,10 @@ fn load_bindings(lua: &Lua, name: &str, read: &Reader) {
match crate::bindings_xml::parse(&crate::source::decode(&bytes)) {
Ok(bindings) => super::keybind::register_addon_bindings(lua, name, &bindings),
// Reported, not swallowed: otherwise a broken binding file leaves keys dead silently.
Err(e) => log_error(lua, &format!("{name}/Bindings.xml: {e}")),
Err(e) => {
let msg = format!("{name}/Bindings.xml: {e}");
log_error(lua, &msg, &msg);
}
}
}
@@ -555,7 +558,8 @@ fn load_saved_variables(lua: &Lua, i: usize) {
Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, // the first-run case
Err(e) => {
// Present but unreadable is held, like a file that will not run.
log_error(lua, &format!("{}: {e}", path.display()));
let msg = format!("{}: {e}", path.display());
log_error(lua, &msg, &msg);
let mut model = lua.app_data_mut::<Model>().expect("model");
if !model.held_saved_files.contains(&path) {
model.held_saved_files.push(path);
@@ -570,7 +574,11 @@ fn load_saved_variables(lua: &Lua, i: usize) {
) {
// Deviation: the reference fails silently; we report it, as a player needs to know,
// and hold the file so the shutdown write keeps it.
log_error(lua, &format!("{}: {e}", path.display()));
log_error(
lua,
&format!("{}: {e}", path.display()),
&super::lua_message(&e),
);
let mut model = lua.app_data_mut::<Model>().expect("model");
if !model.held_saved_files.contains(&path) {
model.held_saved_files.push(path);
@@ -616,7 +624,7 @@ fn run_files(lua: &Lua, name: &str, read: &Reader, files: &[String], toc: &mut S
};
if lua_file {
if let Err(e) = run_chunk(lua, &bytes, &crate::script::addon_chunk_name(name, file)) {
log_error(lua, &format!("{name}/{file}: {e}"));
log_error(lua, &format!("{name}/{file}: {e}"), &super::lua_message(&e));
}
continue;
}
@@ -630,15 +638,15 @@ fn run_files(lua: &Lua, name: &str, read: &Reader, files: &[String], toc: &mut S
continue;
}
};
let report = crate::loader::load_into_under(lua, &doc, &path, &provider, ADDONS_ROOT);
let mut report = crate::loader::load_into_under(lua, &doc, &path, &provider, ADDONS_ROOT);
let banner = crate::status::file_banner(&install);
report.status.close_into(toc, framexml_debug(lua), banner);
std::mem::take(&mut report.status).close_into(toc, framexml_debug(lua), banner);
// Warnings take the `<Addon>/<file>` prefix; a bare loader warning names no document.
for w in report.warnings {
for w in &report.warnings {
crate::script::diagnostics::record_warning(lua, &format!("{name}/{file}: {w}"));
}
for e in report.errors {
log_error(lua, &format!("{name}/{file}: {e}"));
for (e, heard) in report.raised() {
log_error(lua, &format!("{name}/{file}: {e}"), heard);
}
}
}
@@ -683,12 +691,13 @@ fn run_chunk(lua: &Lua, bytes: &[u8], name: &str) -> mlua::Result<()> {
}
/// Record a demand-load error as the startup walk does ([`super::UiScript::report_script_error`]):
/// a `Load` row, dispatched to the player's error handler and put on the host drain.
fn log_error(lua: &Lua, msg: &str) {
/// a `Load` row, dispatched to the player's error handler and put on the host drain. `handler` is
/// the bare Lua message of a raise; a fault that is no raise passes its own text.
fn log_error(lua: &Lua, msg: &str, handler: &str) {
let msg = format!("LoadAddOn: {msg}");
super::diagnostics::record_load_failure(lua, &msg);
let mut model = lua.app_data_mut::<Model>().expect("model");
model.pending_error_dispatch.push(msg.clone());
model.pending_error_dispatch.push(handler.to_string());
model.errors.push(msg);
}
crates/benilla-ui/src/script/button.rs
+1 -1
View File
@@ -998,7 +998,7 @@ pub(super) fn click_button(lua: &Lua, id: u32, button: &str, down: bool, scripte
if let Err(e) = event::fire_widget_handler(lua, id, "OnClick", vec![btn]) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
// A nameplate click selects its unit, from the pointer or `Click()` alike (the plate's
// override `0x7cb910` chains the base), after the handler so an erroring hook cannot eat it. A
crates/benilla-ui/src/script/client.rs
+2 -1
View File
@@ -132,9 +132,10 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
.set_mode(mlua::ChunkMode::Text)
.exec()
{
let bare = super::stdlib::lua_message(&e);
lua.app_data_mut::<crate::script::Model>()
.expect("model")
.record_script_error(super::stdlib::lua_message(&e));
.record_script_error(bare.clone(), bare);
}
Ok(())
})?,
crates/benilla-ui/src/script/colorselect.rs
+1 -1
View File
@@ -268,7 +268,7 @@ fn fire_color_select(lua: &Lua, this: &Table, r: f64, g: f64, b: f64) -> mlua::R
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
Ok(())
}
crates/benilla-ui/src/script/editbox.rs
+2 -2
View File
@@ -294,7 +294,7 @@ fn insert(lua: &Lua, h: FrameHandle, ins: &str, fire_space: bool) {
if let Err(e) = on_char {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
if fire_space {
for _ in 0..out.spaces {
@@ -519,7 +519,7 @@ fn fire_script(lua: &Lua, id: u32, name: &str) {
if let Err(e) = event::fire_widget_handler(lua, id, name, Vec::new()) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
crates/benilla-ui/src/script/editbox/flush.rs
+1 -1
View File
@@ -124,7 +124,7 @@ fn cursor_changed(lua: &Lua, h: FrameHandle) -> bool {
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
true
}
crates/benilla-ui/src/script/event.rs
+2 -2
View File
@@ -38,7 +38,7 @@ pub(super) fn fire_all_event_listeners(lua: &Lua, event: &str, args: &[ScriptVal
if let Err(e) = fire_event_handler(lua, id, event, args) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
at = next;
@@ -102,7 +102,7 @@ pub(super) fn fire_size_changes(lua: &Lua) {
) {
lua.app_data_mut::<Model>()
.expect("model")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
}
crates/benilla-ui/src/script/keyboard.rs
+2 -2
View File
@@ -124,14 +124,14 @@ fn fire(lua: &Lua, id: u32, script: &str, arg: &str) {
Err(e) => {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
return;
}
});
if let Err(e) = event::fire_widget_handler(lua, id, script, vec![val]) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
crates/benilla-ui/src/script/mod.rs
+9 -5
View File
@@ -121,6 +121,8 @@ mod spellbook;
mod stable;
mod statusbar;
mod stdlib;
/// An mlua error's bare Lua message, the one value the error handler is called with.
pub use stdlib::lua_message;
mod summon;
mod talent;
mod taxi;
@@ -1194,14 +1196,16 @@ impl UiScript {
/// Report a script error the host caught outside the VM's dispatch (an addon file that failed
/// to compile or raised at file scope): it joins the handler-dispatch queue and the retained
/// log, not `errors`, since the caller has already logged it.
pub fn report_script_error(&self, msg: &str) {
/// log, not `errors`, since the caller has already logged it. `log` is the decorated text the
/// log keeps; `handler` is the one value the error handler is called with, the bare Lua
/// message as `lua_pcall` leaves it (`0x703b40`).
pub fn report_script_error(&self, log: &str, handler: &str) {
// A Load row, not an Error one: to the player a file scope that raised and a missing file
// are the same fact, the addon is not running.
diagnostics::record_load_failure(&self.lua, msg);
diagnostics::record_load_failure(&self.lua, log);
self.model_mut()
.pending_error_dispatch
.push(msg.to_string());
.push(handler.to_string());
}
/// Hand each queued script error to the Lua error handler, as the reference's catch closure
@@ -1258,7 +1262,7 @@ impl UiScript {
}
fn push_error(&self, e: mlua::Error) {
self.model_mut().record_script_error(e.to_string());
self.model_mut().record_lua_error(&e);
}
}
crates/benilla-ui/src/script/model.rs
+14 -6
View File
@@ -985,13 +985,21 @@ impl Model {
}
}
/// Records a caught script error for the host, the retained log and the Lua error handler; an
/// error raised by the handler itself goes to `errors` directly, which stops the recursion.
pub(crate) fn record_script_error(&mut self, msg: String) {
/// Records a caught script error for the host, the retained log and the Lua error handler: `log`
/// is the decorated text the host and the retained log keep, `handler` the one value the error
/// handler is called with. An error raised by the handler itself goes to `errors` directly,
/// which stops the recursion.
pub(crate) fn record_script_error(&mut self, log: String, handler: String) {
self.diagnostics
.record(super::diagnostics::DiagnosticKind::Error, &msg);
self.pending_error_dispatch.push(msg.clone());
self.errors.push(msg);
.record(super::diagnostics::DiagnosticKind::Error, &log);
self.pending_error_dispatch.push(handler);
self.errors.push(log);
}
/// [`Self::record_script_error`] for an error mlua caught: the log keeps mlua's `Display` (its
/// word and traceback), the handler gets the bare Lua message `lua_pcall` leaves (`0x703b40`).
pub(crate) fn record_lua_error(&mut self, e: &mlua::Error) {
self.record_script_error(e.to_string(), super::stdlib::lua_message(e));
}
/// Records a non-fatal warning for the host's terminal and the retained diagnostic log.
crates/benilla-ui/src/script/object.rs
+7 -6
View File
@@ -600,12 +600,13 @@ pub(super) fn create_frame(
// this call, so the handler can reach the frame by name.
let wrapper = frame_wrapper(lua, id)?;
if let Some(template) = template {
let messages = crate::loader::apply_template(lua, &wrapper, &kind, &template);
if !messages.is_empty() {
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
for m in messages {
model.record_warning(m);
}
let report = crate::loader::apply_template(lua, &wrapper, &kind, &template);
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
for (log, heard) in report.raised() {
model.record_script_error(log.to_string(), heard.to_string());
}
for w in report.warnings {
model.record_warning(w);
}
}
Ok(wrapper)
crates/benilla-ui/src/script/scrollframe.rs
+2 -2
View File
@@ -304,7 +304,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
Ok(())
})?,
@@ -327,7 +327,7 @@ fn fire_scroll(lua: &Lua, this: &Table, script: &str, value: f32) -> mlua::Resul
{
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
Ok(())
}
crates/benilla-ui/src/script/slash.rs
+2 -1
View File
@@ -15,7 +15,8 @@ impl super::UiScript {
// The rest of the line after the alias and one space, spaces intact: the reference's
// `strsub(text, strlen(cmdString) + 2)`.
if let Err(e) = handler.call::<()>(args.to_string()) {
self.model_mut().record_script_error(format!("/{cmd}: {e}"));
self.model_mut()
.record_script_error(format!("/{cmd}: {e}"), super::stdlib::lua_message(&e));
}
true
}
crates/benilla-ui/src/script/slider.rs
+1 -1
View File
@@ -228,7 +228,7 @@ fn fire_value_changed(lua: &Lua, this: &Table, changed: Option<f32>) -> mlua::Re
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
Ok(())
}
crates/benilla-ui/src/script/statusbar.rs
+2 -2
View File
@@ -253,7 +253,7 @@ fn fire_value_changed(lua: &Lua, this: &Table, changed: Option<f32>) -> mlua::Re
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
Ok(())
}
@@ -274,7 +274,7 @@ pub(super) fn fire_engine_value_changed(lua: &Lua, bar: crate::widget::FrameHand
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
crates/benilla-ui/src/script/stdlib.rs
+12 -4
View File
@@ -11,12 +11,20 @@
use mlua::{Lua, Value, Variadic};
/// An mlua error's bare Lua message, as `loadstring` returns it: without the `syntax error: ` or
/// `runtime error: ` word mlua's `Display` adds and the reference never does.
pub(super) fn lua_message(e: &mlua::Error) -> String {
/// An mlua error's bare Lua message: what `lua_pcall` leaves for a 1.12 error function (`0x703b40`)
/// and what `loadstring` returns. It drops the `syntax error: ` and `runtime error: ` words mlua's
/// `Display` adds, the `stack traceback:` block mlua's message handler appends to a raise, and the
/// wrappers a Rust callback's failure arrives in.
pub fn lua_message(e: &mlua::Error) -> String {
match e {
mlua::Error::SyntaxError { message, .. } => message.clone(),
mlua::Error::RuntimeError(m) => m.clone(),
mlua::Error::RuntimeError(m) => m
.rfind("\nstack traceback:\n")
.map_or(m.as_str(), |at| &m[..at])
.to_string(),
mlua::Error::CallbackError { cause, .. } | mlua::Error::WithContext { cause, .. } => {
lua_message(cause)
}
other => other.to_string(),
}
}
crates/benilla-ui/src/script/tests/create_frame_template.rs
+38
View File
@@ -464,3 +464,41 @@ fn a_runtime_template_fires_its_onshow_after_its_onload_when_visible() {
);
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// A template's OnLoad runs under the error function as a file's does (`CreateFrame` builds through
/// `0x6ee280`, OnLoad through `0x704d50`): a raise reaches the chosen error handler as the bare
/// message, and `CreateFrame` still returns the frame.
#[test]
fn a_raising_template_onload_reaches_the_error_handler() {
let mut s = script();
s.set_screen_size(800.0, 600.0);
register(
&s,
r#"<Ui>
<Frame name="RaisingTemplate" virtual="true">
<Scripts><OnLoad>this:NoSuchMethod()</OnLoad></Scripts>
</Frame>
</Ui>"#,
);
s.run(
"caught = {} seterrorhandler(function(m) table.insert(caught, m) end) \
made = CreateFrame('Frame', 'Raised', UIParent, 'RaisingTemplate')",
)
.unwrap();
s.dispatch_script_errors_to_handler();
let caught: Vec<String> = s.eval("return caught").unwrap();
assert_eq!(caught.len(), 1, "{caught:?}");
assert!(
caught[0].ends_with("attempt to call method `NoSuchMethod' (a nil value)")
&& !caught[0].contains('\n'),
"{:?}",
caught[0]
);
let made: bool = s
.eval("return made ~= nil and made:GetName() == 'Raised'")
.unwrap();
assert!(
made,
"CreateFrame returns the frame after its OnLoad raised"
);
}
crates/benilla-ui/src/script/tests/mod.rs
+45
View File
@@ -221,6 +221,51 @@ fn a_widget_handler_error_reaches_the_chosen_error_handler() {
assert!(caught.contains("slide boom"), "OnValueChanged: {caught}");
}
/// 1.12 calls the error handler with the one value the failed call raised (`0x703b40`: the
/// handler pushed, inserted below the message, `lua_call` 1 argument): the bare Lua message, no
/// `runtime error: ` word and no traceback, which only benilla's own log carries.
#[test]
fn the_error_handler_receives_the_bare_lua_message() {
let mut s = crate::script::UiScript::new().unwrap();
s.set_screen_size(1024.0, 768.0);
s.run(
"caught = {} seterrorhandler(function(m) table.insert(caught, m) end) \
local f = CreateFrame('Frame') \
f:RegisterEvent('ERROR_PROBE') \
f:SetScript('OnEvent', function() this:NoSuchMethod() end) \
local b = CreateFrame('Button') \
b:SetScript('OnClick', function() local t = nil; t.x = 1 end) \
b:Click() \
local c = CreateFrame('Button') \
c:SetScript('OnClick', function() SetPortraitTexture() end) \
c:Click()",
)
.unwrap();
s.fire_event("ERROR_PROBE", vec![]);
s.dispatch_script_errors_to_handler();
let caught: Vec<String> = s.eval("return caught").unwrap();
assert!(caught.len() >= 3, "{caught:?}");
for m in &caught {
assert!(
!m.contains("runtime error") && !m.contains("stack traceback") && !m.contains('\n'),
"bare message, one line: {m:?}"
);
}
for want in [
"attempt to call method `NoSuchMethod' (a nil value)",
":1: attempt to index local `t' (a nil value)",
] {
assert!(
caught
.iter()
.any(|m| m.starts_with("[string \"") && m.ends_with(want)),
"{want}: {caught:?}"
);
}
// The host log keeps the decorated text.
assert!(s.take_errors().iter().any(|e| e.contains("runtime error")));
}
/// `fire_global` carries `UPDATE_FACTION` from the faction-header verbs.
#[test]
fn a_self_unregistering_listener_does_not_rob_its_successor_on_the_internal_dispatch() {
crates/benilla-ui/src/script/tick.rs
+1 -1
View File
@@ -61,7 +61,7 @@ pub(crate) fn fire_event_into(lua: &Lua, event: &str, args: Vec<ScriptValue>) {
let id = model.frame_id(h);
drop(model);
if let Err(e) = event::fire_event_handler(lua, id, event, &args) {
model_mut().record_script_error(e.to_string());
model_mut().record_lua_error(&e);
}
at = next;
}
crates/benilla-ui/src/script/tooltip/mod.rs
+1 -1
View File
@@ -313,7 +313,7 @@ pub(super) fn fire_cleared(lua: &Lua, h: FrameHandle) {
if let Err(e) = event::fire_widget_handler(lua, id, "OnTooltipCleared", Vec::new()) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
crates/benilla-ui/src/script/tooltip_item/mod.rs
+1 -1
View File
@@ -114,7 +114,7 @@ fn fire_add_money(lua: &Lua, h: crate::widget::FrameHandle, copper: u64) {
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
crates/benilla-ui/src/script/tooltip_unit.rs
+1 -1
View File
@@ -226,7 +226,7 @@ fn update_bar(lua: &Lua, h: FrameHandle, unit: Option<&UnitState>) {
) {
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
.record_lua_error(&e);
}
}
}
crates/benilla-ui/src/script/visibility.rs
+1 -1
View File
@@ -152,6 +152,6 @@ fn model_mut(lua: &Lua) -> mlua::AppDataRefMut<'_, Model> {
/// A handler error is recorded in [`Model::errors`], never propagated to the verb.
fn record(lua: &Lua, fired: mlua::Result<()>) {
if let Err(e) = fired {
model_mut(lua).record_script_error(e.to_string());
model_mut(lua).record_lua_error(&e);
}
}