From 58a31a1b329e922dc86c554e8220012ab7238f1b Mon Sep 17 00:00:00 2001 From: aaron <83140718+centau@users.noreply.github.com> Date: Fri, 27 Dec 2024 22:43:01 +0000 Subject: [PATCH] Try improve error reporting --- CHANGELOG.md | 2 + src/apply.luau | 5 +- src/batch.luau | 5 +- src/cleanup.luau | 5 +- src/context.luau | 7 ++- src/create.luau | 7 ++- src/graph.luau | 31 +++++++---- src/lib.luau | 5 +- src/maps.luau | 9 ++-- src/root.luau | 8 ++- src/source.luau | 4 +- src/spring.luau | 7 +-- src/switch.luau | 5 +- src/throw.luau | 5 -- src/untrack.luau | 4 +- test/stacktrace-test.luau | 110 ++++++++++++++++++++++++++++++++++++++ 16 files changed, 164 insertions(+), 55 deletions(-) delete mode 100644 src/throw.luau create mode 100644 test/stacktrace-test.luau diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c6303b..ef49bb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). for this. - Implicit effects to set children now unparent all children when the effect is destroyed. +- Error reporting should be improved with better formatting when effects invoke + other effects and no more loss of stack traces. ### Removed diff --git a/src/apply.luau b/src/apply.luau index 2b30ba6..95645da 100644 --- a/src/apply.luau +++ b/src/apply.luau @@ -1,7 +1,6 @@ local typeof = game and typeof or require "../test/mock".typeof :: never local flags = require "./flags" -local throw = require "./throw" local implicit_effect = require "./implicit_effect" local _, is_action = require "./action"() local graph = require "./graph" @@ -67,7 +66,7 @@ local function process_properties(properties: Map, instance: I if type(property) == "string" then if flags.strict then -- check for duplicate property assignment at nesting depth if cache.nested_debug[depth][property] then - throw(`duplicate property {property} at depth {depth}`) + error(`duplicate property {property} at depth {depth}`, 0) end cache.nested_debug[depth][property] = true end @@ -104,7 +103,7 @@ end -- applies table of nested properties to an instance using full vide semantics local function apply(instance: T & Instance, properties: { [unknown]: unknown }): T if not properties then - throw("attempt to call a constructor returned by create() with no properties") + error "attempt to call a constructor returned by create() with no properties" end -- queue parent assignment if any for last diff --git a/src/batch.luau b/src/batch.luau index 4cd7581..e3e6d40 100644 --- a/src/batch.luau +++ b/src/batch.luau @@ -1,5 +1,4 @@ local flags = require "./flags" -local throw = require "./throw" local graph = require "./graph" local function batch(setter: () -> ()) @@ -11,14 +10,14 @@ local function batch(setter: () -> ()) from = graph.get_update_queue_length() end - local ok, err: string? = pcall(setter) + local ok, err: string? = xpcall(setter, debug.traceback) if not already_batching then flags.batch = false graph.flush_update_queue(from) end - if not ok then throw(`error occured while batching updates: {err}`) end + if not ok then error(`error occured while batching updates: {err}`, 0) end end return batch diff --git a/src/cleanup.luau b/src/cleanup.luau index 20d188b..6c46bbf 100644 --- a/src/cleanup.luau +++ b/src/cleanup.luau @@ -1,6 +1,5 @@ local typeof = game and typeof or require "../test/mock".typeof :: never -local throw = require "./throw" local graph = require "./graph" local get_scope = graph.get_scope local push_cleanup = graph.push_cleanup @@ -14,14 +13,14 @@ local function helper(obj: any) elseif obj.disconnect then function() obj:disconnect() end elseif obj.Destroy then function() obj:Destroy() end elseif obj.Disconnect then function() obj:Disconnect() end - else throw("cannot cleanup given object") + else error "cannot cleanup given object" end local function cleanup(value: unknown) local scope = get_scope() if not scope then - throw "cannot cleanup outside a stable or reactive scope" + error "cannot cleanup outside a stable or reactive scope" end; assert(scope) if type(value) == "function" then diff --git a/src/context.luau b/src/context.luau index a46ad5a..2ce3eff 100644 --- a/src/context.luau +++ b/src/context.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local graph = require "./graph" type Node = graph.Node local create_node = graph.create_node @@ -44,10 +43,10 @@ local function context(...: T): Context if has_default ~= nil then return default_value else - throw("attempt to get context when no context is set and no default context is set") + error("attempt to get context when no context is set and no default context is set", 0) end else -- set - if not scope then return throw("attempt to set context outside of a vide scope") end + if not scope then return error("attempt to set context outside of a vide scope", 0) end local value, component = ... @@ -62,7 +61,7 @@ local function context(...: T): Context pop_scope() if not ok then - throw(`error while running context:\n\n{result}`) + error(`error while running context:\n\n{result}`, 0) end return result diff --git a/src/create.luau b/src/create.luau index dea8ca7..ebcbf54 100644 --- a/src/create.luau +++ b/src/create.luau @@ -1,7 +1,6 @@ local typeof = game and typeof or require "../test/mock".typeof :: never local Instance = game and Instance or require "../test/mock".Instance :: never -local throw = require "./throw" local defaults = require "./defaults" local apply = require "./apply" @@ -10,7 +9,7 @@ local ctor_cache = {} :: { [string]: () -> Instance } setmetatable(ctor_cache :: any, { __index = function(self, class) local ok, instance: Instance = pcall(Instance.new, class :: any) - if not ok then throw(`invalid class name, could not create instance of class { class }`) end + if not ok then error(`invalid class name, could not create instance of class { class }`, 0) end local default: { [string]: unknown }? = defaults[class] if default then @@ -35,7 +34,7 @@ end local function clone_instance(instance: Instance) return function(properties: Props): Instance local clone = instance:Clone() - if not clone then throw "attempt to clone a non-archivable instance" end + if not clone then error "attempt to clone a non-archivable instance" end return apply(clone, properties) end end @@ -47,7 +46,7 @@ local function create(class_or_instance: string | Instance, props: Props?): ((Pr elseif typeof(class_or_instance) == "Instance" then result = clone_instance(class_or_instance) else - throw("bad argument #1, expected string or instance, got " .. typeof(class_or_instance)) + error("bad argument #1, expected string or instance, got " .. typeof(class_or_instance), 0) return nil :: never end if props then diff --git a/src/graph.luau b/src/graph.luau index be58b15..360c0cb 100644 --- a/src/graph.luau +++ b/src/graph.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local flags = require "./flags" export type SourceNode = { @@ -22,9 +21,23 @@ export type Node = { local scopes = { n = 0 } :: { [number]: Node, n: number } -- scopes stack +local function efn(err: string) + local trace = debug.traceback(err, 2) + + if string.find(err, "^effect error stacktrace") then -- if effect error is nested + trace = string.gsub(" " .. trace, "\n", function() -- indent entire error + return "\n " + end) + end + + trace ..= "\nsource update stacktrace:" +return trace +end + local function ycall(fn: (T) -> U, arg: T): (boolean, string|U) + local thread = coroutine.create(xpcall) - local function efn(err: string) return debug.traceback(err, 3) end + --local function efn(err: string) return debug.traceback(err, 3) end local resume_ok, run_ok, result = coroutine.resume(thread, fn, efn, arg) assert(resume_ok) @@ -45,9 +58,9 @@ local function assert_stable_scope(): Node if not scope then local caller_name = debug.info(2, "n") - return throw(`cannot use {caller_name}() outside a stable or reactive scope`) + return error(`cannot use {caller_name}() outside a stable or reactive scope`, 0) elseif scope.effect then - throw("cannot create a new reactive scope inside another reactive scope") + error("cannot create a new reactive scope inside another reactive scope", 0) end return scope @@ -81,8 +94,8 @@ end local function flush_cleanups(node: Node) if node.cleanups then for _, fn in next, node.cleanups do - local ok, err: string? = pcall(fn) - if not ok then throw(`cleanup error: {err}`) end + local ok, err: string? = xpcall(fn, debug.traceback) + if not ok then error(`cleanup error: {err}`, 0) end end table.clear(node.cleanups) @@ -107,7 +120,7 @@ end local function destroy(node: Node) if flags.strict and table.find(scopes, node) then - throw("attempt to destroy an active scope") + error("attempt to destroy an active scope", 0) end flush_cleanups(node) @@ -150,7 +163,7 @@ local function evaluate_node(node: Node) if not ok then table.clear(update_queue) update_queue.n = 0 - throw(`effect stacktrace:\n{new_value :: string}`) + error(`effect error stacktrace\n{new_value :: string}`, 0) end node.cache = new_value :: T @@ -170,7 +183,7 @@ local function evaluate_node(node: Node) if not ok then table.clear(update_queue) update_queue.n = 0 - throw(`effect stacktrace:\n{new_value}\n`) + error(`effect error:\n{new_value}\n`, 0) end node.cache = new_value diff --git a/src/lib.luau b/src/lib.luau index a54bd32..3d7f3be 100644 --- a/src/lib.luau +++ b/src/lib.luau @@ -18,7 +18,6 @@ local indexes, values = require "./maps"() local spring, update_springs = require "./spring"() local action = require "./action"() local changed = require "./changed" -local throw = require "./throw" local flags = require "./flags" export type Source = source.Source @@ -98,7 +97,7 @@ local vide = { setmetatable(vide :: any, { __index = function(_, index: unknown): () if flags[index] == nil then - throw(`{tostring(index)} is not a valid member of vide`) + error(`{tostring(index)} is not a valid member of vide`, 0) else return flags[index] end @@ -106,7 +105,7 @@ setmetatable(vide :: any, { __newindex = function(_, index: unknown, value: unknown) if flags[index] == nil then - throw(`{tostring(index)} is not a valid member of vide`) + error(`{tostring(index)} is not a valid member of vide, 0`) else flags[index] = value end diff --git a/src/maps.luau b/src/maps.luau index bd59a6c..0f95695 100644 --- a/src/maps.luau +++ b/src/maps.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local flags = require "./flags" local graph = require "./graph" type Node = graph.Node @@ -20,7 +19,7 @@ local function check_primitives(t: {}) for _, v in next, t do if type(v) == "table" or type(v) == "userdata" or type(v) == "function" then continue end - throw("table source map cannot return primitives") + error("table source map cannot return primitives", 0) end end @@ -71,7 +70,7 @@ local function indexes(input: () -> Map, transform: (() -> VI, push_scope(scope) - local ok, result = pcall(transform, function() + local ok, result = xpcall(transform, debug.traceback, function() push_child_to_scope(node) return node.cache end, i) @@ -132,7 +131,7 @@ local function values(input: () -> Map, transform: (VI, () -> local cache = {} for _, v in next, data do if cache[v] ~= nil then - throw "duplicate table value detected" + error "duplicate table value detected" end cache[v] = true end @@ -154,7 +153,7 @@ local function values(input: () -> Map, transform: (VI, () -> push_scope(scope) - local ok, result = pcall(transform, v, function() + local ok, result = xpcall(transform, debug.traceback, v, function() push_child_to_scope(node) return node.cache end) diff --git a/src/root.luau b/src/root.luau index d8dc230..bc9c2fc 100644 --- a/src/root.luau +++ b/src/root.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local graph = require "./graph" type Node = graph.Node local create_node = graph.create_node @@ -14,21 +13,20 @@ local function root(fn: (destroy: () -> ()) -> T...): (() -> (), T...) refs[node] = true -- prevent gc of root node local destroy = function() - if not refs[node] then throw "root already destroyed" end + if not refs[node] then error "root already destroyed" end refs[node] = nil destroy(node) end push_scope(node) - local function efn(err: string) return debug.traceback(err, 3) end - local result = { xpcall(fn, efn, destroy) } + local result = { xpcall(fn, debug.traceback, destroy) } pop_scope() if not result[1] then destroy() - throw(`error while running root():\n\n{result[2]}`) + error(`error while running root():\n\n{result[2]}`, 0) end return destroy, unpack(result :: any, 2) diff --git a/src/source.luau b/src/source.luau index 0b89637..d7aa53d 100644 --- a/src/source.luau +++ b/src/source.luau @@ -9,7 +9,7 @@ export type Source = (() -> T) & ((value: T) -> T) local function source(initial_value: T): Source local node = create_source_node(initial_value) - return function(...): T + local function update_source(...): T if select("#", ...) == 0 then -- no args were given push_child_to_scope(node) return node.cache @@ -24,6 +24,8 @@ local function source(initial_value: T): Source update_descendants(node) return v end + + return update_source end return source :: ((initial_value: T) -> Source) & (() -> Source) diff --git a/src/spring.luau b/src/spring.luau index 053c945..aaf6789 100644 --- a/src/spring.luau +++ b/src/spring.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local graph = require "./graph" type Node = graph.Node type SourceNode = graph.SourceNode @@ -114,7 +113,7 @@ local vec6_to_type = { local invalid_type = { __index = function(_, t: string) - throw(`cannot spring type {t}`) + error(`cannot spring type {t}`, 0) end } @@ -141,7 +140,7 @@ local function spring(source: () -> T, period: number?, damping_ratio: number -- todo: is there a solution other than reducing step size? -- todo: this does not catch all solver exploding cases if c > UPDATE_RATE*2 then -- solver will explode if this is true - throw("spring damping too high, consider reducing damping or increasing period") + error("spring damping too high, consider reducing damping or increasing period", 0) end local data: SpringState = { @@ -263,8 +262,6 @@ local function step_springs(dt: number) end end -local remove_queue = {} - local function update_spring_sources() for data, output in springs do local x0_123, x1_123, v_123, diff --git a/src/switch.luau b/src/switch.luau index cb7cba4..547fc80 100644 --- a/src/switch.luau +++ b/src/switch.luau @@ -1,4 +1,3 @@ -local throw = require "./throw" local graph = require "./graph" type Node = graph.Node type SourceNode = graph.SourceNode @@ -32,7 +31,7 @@ local function switch(source: () -> T): (map: Map U)?)>) -> () if component == nil then return nil end if type(component) ~= "function" then - throw "map must map a value to a function" + error "map must map a value to a function" end local new_scope = create_node(owner, false, false) @@ -40,7 +39,7 @@ local function switch(source: () -> T): (map: Map U)?)>) -> () push_scope(new_scope) - local ok, result = pcall(component) + local ok, result = xpcall(component, debug.traceback) pop_scope() diff --git a/src/throw.luau b/src/throw.luau deleted file mode 100644 index 4135f69..0000000 --- a/src/throw.luau +++ /dev/null @@ -1,5 +0,0 @@ -local function VIDE_ASSERT(msg): any - error(msg, 0) -end - -return VIDE_ASSERT diff --git a/src/untrack.luau b/src/untrack.luau index 90ceee2..577da25 100644 --- a/src/untrack.luau +++ b/src/untrack.luau @@ -10,13 +10,13 @@ local function untrack(source: () -> T): T local effect = scope.effect scope.effect = false - local ok, result = pcall(source) + local ok, result = xpcall(source, debug.traceback) scope.effect = effect :: () -> () if not ok then error(result, 0) end - return result + return result :: T else return source() end diff --git a/test/stacktrace-test.luau b/test/stacktrace-test.luau new file mode 100644 index 0000000..e24c314 --- /dev/null +++ b/test/stacktrace-test.luau @@ -0,0 +1,110 @@ +local vide = require "../" + +do + print "=============================================================" + + local a = vide.source(1) + + local cause_error = false + + local function try_error() + if cause_error then error("uh oh") end + end + + vide.root(function() + vide.effect(function() + a() + try_error() + end) + end) + + cause_error = true + + local ok, result = pcall(function() a(2) end) + print(result) + + print "=============================================================" +end + +do + print "=============================================================" + + local a = vide.source(1) + local b = vide.source(1) + local c = vide.source(1) + + local cause_error = false + + local function try_error() + if cause_error then error("uh oh") end + end + + vide.root(function() + vide.effect(function() + a() + b(vide.untrack(b) + 1) + end) + + vide.effect(function() + b() + c(vide.untrack(c) + 1) + end) + + + vide.effect(function() + c() + try_error() + end) + end) + + cause_error = true + + local ok, result = pcall(function() a(2) end) + print(result) + + print "=============================================================" +end + +do + print "=============================================================" + + local a = vide.source(1) + local b = vide.source(1) + local c = vide.source(1) + + local cause_error = false + + local function try_error() + if cause_error then error("uh oh") end + end + + vide.root(function() + vide.effect(function() + a() + vide.untrack(function() -- todo: this trace appearing twice + b(b() + 1) + return nil + end) + end) + + vide.effect(function() + b() + vide.batch(function() + c(vide.untrack(c) + 1) + end) + end) + + + vide.effect(function() + c() + try_error() + end) + end) + + cause_error = true + + local ok, result = pcall(function() a(2) end) + print(result) + + print "=============================================================" +end