From 801da61c0fbaf2e35e4993537b892a10a96208d5 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Mon, 20 Feb 2023 13:02:22 +0100 Subject: [PATCH] Clean up error handling & chunk loading --- packages/lib/src/globals/require.rs | 40 +++++++++++++------------- packages/lib/src/globals/task.rs | 16 +++++------ packages/lib/src/lua/async_ext.rs | 39 ++++++++----------------- packages/lib/src/lua/create.rs | 19 ------------ packages/lib/src/lua/task/scheduler.rs | 27 ++++++++++------- packages/lib/src/utils/formatting.rs | 2 ++ 6 files changed, 58 insertions(+), 85 deletions(-) diff --git a/packages/lib/src/globals/require.rs b/packages/lib/src/globals/require.rs index 938b4f3..33f5843 100644 --- a/packages/lib/src/globals/require.rs +++ b/packages/lib/src/globals/require.rs @@ -8,6 +8,25 @@ use mlua::prelude::*; use crate::utils::table::TableBuilder; +const REQUIRE_IMPL_LUA: &str = r#" +local source = info(1, "s") +if source == '[string "require"]' then + source = info(2, "s") +end +local absolute, relative = paths(source, ...) +if loaded[absolute] ~= true then + local first, second = load(absolute, relative) + if first == nil or second ~= nil then + error("Module did not return exactly one value") + end + loaded[absolute] = true + cache[absolute] = first + return first +else + return cache[absolute] +end +"#; + pub fn create(lua: &'static Lua) -> LuaResult { // Preserve original require behavior if we have a special env var set, // returning an empty table since there are no globals to overwrite @@ -89,26 +108,7 @@ pub fn create(lua: &'static Lua) -> LuaResult { .with_value("load", require_get_loaded_file)? .build_readonly()?; let require_fn_lua = lua - .load( - r#" - local source = info(1, "s") - if source == '[string "require"]' then - source = info(2, "s") - end - local absolute, relative = paths(source, ...) - if loaded[absolute] ~= true then - local first, second = load(absolute, relative) - if first == nil or second ~= nil then - error("Module did not return exactly one value") - end - loaded[absolute] = true - cache[absolute] = first - return first - else - return cache[absolute] - end - "#, - ) + .load(REQUIRE_IMPL_LUA) .set_name("require")? .set_environment(require_env)? .into_function()?; diff --git a/packages/lib/src/globals/task.rs b/packages/lib/src/globals/task.rs index 9907876..f18831f 100644 --- a/packages/lib/src/globals/task.rs +++ b/packages/lib/src/globals/task.rs @@ -11,6 +11,13 @@ use crate::{ utils::table::TableBuilder, }; +const SPAWN_IMPL_LUA: &str = r#" +scheduleNext(thread()) +local task = scheduleNext(...) +yield() +return task +"#; + pub fn create(lua: &'static Lua) -> LuaResult> { lua.app_data_ref::<&TaskScheduler>() .expect("Missing task scheduler in app data"); @@ -27,14 +34,7 @@ pub fn create(lua: &'static Lua) -> LuaResult> { */ let task_spawn_env_yield: LuaFunction = lua.named_registry_value("co.yield")?; let task_spawn = lua - .load( - " - scheduleNext(thread()) - local task = scheduleNext(...) - yield() - return task - ", - ) + .load(SPAWN_IMPL_LUA) .set_name("task.spawn")? .set_environment( TableBuilder::new(lua)? diff --git a/packages/lib/src/lua/async_ext.rs b/packages/lib/src/lua/async_ext.rs index e90e5d9..628f630 100644 --- a/packages/lib/src/lua/async_ext.rs +++ b/packages/lib/src/lua/async_ext.rs @@ -6,6 +6,16 @@ use crate::{lua::task::TaskScheduler, utils::table::TableBuilder}; use super::task::TaskSchedulerAsyncExt; +const ASYNC_IMPL_LUA: &str = r#" +resumeAsync(thread(), ...) +return yield() +"#; + +const WAIT_IMPL_LUA: &str = r#" +resumeAfter(...) +return yield() +"#; + #[async_trait(?Send)] pub trait LuaAsyncExt { fn create_async_function<'lua, A, R, F, FR>(self, func: F) -> LuaResult> @@ -30,18 +40,8 @@ impl LuaAsyncExt for &'static Lua { F: 'static + Fn(&'static Lua, A) -> FR, FR: 'static + Future>, { - let async_env_make_err: LuaFunction = self.named_registry_value("dbg.makeerr")?; - let async_env_is_err: LuaFunction = self.named_registry_value("dbg.iserr")?; - let async_env_trace: LuaFunction = self.named_registry_value("dbg.trace")?; - let async_env_error: LuaFunction = self.named_registry_value("error")?; - let async_env_unpack: LuaFunction = self.named_registry_value("tab.unpack")?; let async_env_yield: LuaFunction = self.named_registry_value("co.yield")?; let async_env = TableBuilder::new(self)? - .with_value("makeError", async_env_make_err)? - .with_value("isError", async_env_is_err)? - .with_value("trace", async_env_trace)? - .with_value("error", async_env_error)? - .with_value("unpack", async_env_unpack)? .with_value("yield", async_env_yield)? .with_function("thread", |lua, _: ()| Ok(lua.current_thread()))? .with_function( @@ -60,17 +60,7 @@ impl LuaAsyncExt for &'static Lua { )? .build_readonly()?; let async_func = self - .load( - " - resumeAsync(thread(), ...) - local results = { yield() } - if isError(results[1]) then - error(makeError(results[1], trace())) - else - return unpack(results) - end - ", - ) + .load(ASYNC_IMPL_LUA) .set_name("async")? .set_environment(async_env)? .into_function()?; @@ -94,12 +84,7 @@ impl LuaAsyncExt for &'static Lua { })? .build_readonly()?; let async_func = self - .load( - " - resumeAfter(...) - return yield() - ", - ) + .load(WAIT_IMPL_LUA) .set_name("wait")? .set_environment(async_env)? .into_function()?; diff --git a/packages/lib/src/lua/create.rs b/packages/lib/src/lua/create.rs index 3aff66d..4d76ad0 100644 --- a/packages/lib/src/lua/create.rs +++ b/packages/lib/src/lua/create.rs @@ -48,9 +48,6 @@ end These globals can then be modified safely after constructing Lua using this function. - --- - * `"require"` -> `require` - * `"select"` -> `select` --- * `"print"` -> `print` * `"error"` -> `error` @@ -82,8 +79,6 @@ pub fn create() -> LuaResult<&'static Lua> { let coroutine: LuaTable = globals.get("coroutine")?; // Store original lua global functions in the registry so we can use // them later without passing them around and dealing with lifetimes - lua.set_named_registry_value("require", globals.get::<_, LuaFunction>("require")?)?; - lua.set_named_registry_value("select", globals.get::<_, LuaFunction>("select")?)?; lua.set_named_registry_value("print", globals.get::<_, LuaFunction>("print")?)?; lua.set_named_registry_value("error", globals.get::<_, LuaFunction>("error")?)?; lua.set_named_registry_value("type", globals.get::<_, LuaFunction>("type")?)?; @@ -98,18 +93,6 @@ pub fn create() -> LuaResult<&'static Lua> { lua.set_named_registry_value("dbg.info", debug.get::<_, LuaFunction>("info")?)?; lua.set_named_registry_value("tab.pack", table.get::<_, LuaFunction>("pack")?)?; lua.set_named_registry_value("tab.unpack", table.get::<_, LuaFunction>("unpack")?)?; - // Create a function that can be called from lua to check if a value is a mlua error, - // this will be used in async environments for proper error handling and throwing, as - // well as a function that can be called to make a callback error with a traceback from lua - let dbg_is_err_fn = - lua.create_function(move |_, value: LuaValue| Ok(matches!(value, LuaValue::Error(_))))?; - - let dbg_make_err_fn = lua.create_function(|_, (cause, traceback): (LuaError, String)| { - Ok(LuaError::CallbackError { - traceback, - cause: cause.into(), - }) - })?; // Create a trace function that can be called to obtain a full stack trace from // lua, this is not possible to do from rust when using our manual scheduler let dbg_trace_env = lua.create_table_with_capacity(0, 1)?; @@ -123,8 +106,6 @@ pub fn create() -> LuaResult<&'static Lua> { .set_environment(dbg_trace_env)? .into_function()?; lua.set_named_registry_value("dbg.trace", dbg_trace_fn)?; - lua.set_named_registry_value("dbg.iserr", dbg_is_err_fn)?; - lua.set_named_registry_value("dbg.makeerr", dbg_make_err_fn)?; // All done Ok(lua) } diff --git a/packages/lib/src/lua/task/scheduler.rs b/packages/lib/src/lua/task/scheduler.rs index a0499f6..e2f5e26 100644 --- a/packages/lib/src/lua/task/scheduler.rs +++ b/packages/lib/src/lua/task/scheduler.rs @@ -3,6 +3,7 @@ use std::{ cell::{Cell, RefCell}, collections::{HashMap, VecDeque}, process::ExitCode, + sync::Arc, }; use futures_util::{future::LocalBoxFuture, stream::FuturesUnordered, Future}; @@ -45,6 +46,7 @@ pub struct TaskScheduler<'fut> { pub(super) tasks: RefCell>, pub(super) tasks_current: Cell>, pub(super) tasks_queue_blocking: RefCell>, + pub(super) tasks_current_lua_error: Arc>>, // Future tasks & objects for waking pub(super) futures: AsyncMutex>>, pub(super) futures_registered_count: Cell, @@ -58,6 +60,12 @@ impl<'fut> TaskScheduler<'fut> { */ pub fn new(lua: &'static Lua) -> LuaResult { let (tx, rx) = mpsc::unbounded_channel(); + let tasks_current_lua_error = Arc::new(RefCell::new(None)); + let tasks_current_lua_error_inner = tasks_current_lua_error.clone(); + lua.set_interrupt(move || match tasks_current_lua_error_inner.take() { + Some(err) => Err(err), + None => Ok(LuaVmState::Continue), + }); Ok(Self { lua, guid: Cell::new(0), @@ -65,6 +73,7 @@ impl<'fut> TaskScheduler<'fut> { tasks: RefCell::new(HashMap::new()), tasks_current: Cell::new(None), tasks_queue_blocking: RefCell::new(VecDeque::new()), + tasks_current_lua_error, futures: AsyncMutex::new(FuturesUnordered::new()), futures_tx: tx, futures_rx: AsyncMutex::new(rx), @@ -268,17 +277,13 @@ impl<'fut> TaskScheduler<'fut> { self.tasks_current.set(Some(reference)); let rets = match args_opt_res { Some(args_res) => match args_res { - /* - HACK: Resuming with an error here only works because the Rust - functions that we register and that may return lua errors are - also error-aware and wrapped in a special wrapper that checks - if the returned value is a lua error userdata, then throws it - - Also note that this only happens for our custom async functions - that may pass errors as arguments when resuming tasks, other - native mlua functions will handle this and dont need wrapping - */ - Err(e) => thread.resume(e), + Err(e) => { + // NOTE: Setting this error here means that when the thread + // is resumed it will error instantly, so we don't need + // to call it with proper args, empty args is fine + *self.tasks_current_lua_error.borrow_mut() = Some(e); + thread.resume(()) + } Ok(args) => thread.resume(args), }, None => thread.resume(()), diff --git a/packages/lib/src/utils/formatting.rs b/packages/lib/src/utils/formatting.rs index e15395e..f843488 100644 --- a/packages/lib/src/utils/formatting.rs +++ b/packages/lib/src/utils/formatting.rs @@ -426,6 +426,8 @@ fn fix_error_nitpicks(full_message: String) -> String { .replace("'require', Line 5", "'[C]' - function require") .replace("'require', Line 7", "'[C]' - function require") .replace("'require', Line 8", "'[C]' - function require") + // Same thing here for our async script + .replace("'async', Line 3", "'[C]'") // Fix error calls in custom script chunks coming through .replace( "'[C]' - function error\n Script '[C]' - function require",