diff --git a/src/graph.luau b/src/graph.luau index b85b66e..be76dc1 100644 --- a/src/graph.luau +++ b/src/graph.luau @@ -98,15 +98,20 @@ local function push_cleanup(node: Node, cleanup: () -> ()) end end -local function flush_cleanups(node: Node) +-- runs every cleanup even if one errors, returns the first error +local function flush_cleanups(node: Node): string? + local first_err: string? + if node.cleanups then for _, fn in node.cleanups do local ok, err: string? = xpcall(fn, debug.traceback) - if not ok then error(`cleanup error: {err}`, 0) end + if not ok and not first_err then first_err = `cleanup error: {err}` end end table.clear(node.cleanups) end + + return first_err end local function find_and_swap_pop(t: { T }, v: T) @@ -125,12 +130,15 @@ local function unparent(node: Node) end end -local function destroy(node: Node) +local destroy_owned: (node: Node) -> string? + +-- always fully destroys the node, returns the first cleanup error +local function destroy_node(node: Node): string? if flags.strict and table.find(scopes, node) then error("attempt to destroy an active scope", 0) end - flush_cleanups(node) + local err = flush_cleanups(node) unparent(node) if node.owner then @@ -138,70 +146,85 @@ local function destroy(node: Node) node.owner = false end - if node.owned then - local owned = node.owned - while owned[1] do destroy(owned[1]) end - end + local owned_err = destroy_owned(node) + return err or owned_err end -local function destroy_owned(node: Node) +function destroy_owned(node: Node): string? + local err: string? + if node.owned then local owned = node.owned - while owned[1] do destroy(owned[1]) end + while owned[1] do + local owned_err = destroy_node(owned[1]) + err = err or owned_err + end end + + return err +end + +local function destroy(node: Node) + local err = destroy_node(node) + if err then error(err, 0) end end local update_queue = { n = 0 } :: { n: number, [number]: Node } -local function evaluate_node(node: Node) +-- returns whether the cached value changed, and an error instead of throwing +local function try_evaluate_node(node: Node): (boolean, string?) if flags.strict then if table.find(scopes, node) then - error("a scope, that should rerun due to the update of a source, is already active", 0) + return false, "a scope, that should rerun due to the update of a source, is already active" end local initial_value = node.cache + local err: string? for i = 1, 2 do local cur_value = node.cache - flush_cleanups(node) - destroy_owned(node) + local cleanup_err = flush_cleanups(node) + local owned_err = destroy_owned(node) + err = err or cleanup_err or owned_err push_scope(node) local ok, new_value = ycall(node.effect :: (T) -> T, cur_value) pop_scope() if not ok then - table.clear(update_queue) - update_queue.n = 0 - error(`effect error stacktrace\n{new_value :: string}`, 0) + return false, `effect error stacktrace\n{new_value :: string}` end node.cache = new_value :: T end - return initial_value ~= node.cache + return initial_value ~= node.cache, err else local cur_value = node.cache - flush_cleanups(node) - destroy_owned(node) + local cleanup_err = flush_cleanups(node) + local owned_err = destroy_owned(node) push_scope(node) local ok, new_value = xpcall(node.effect :: (T) -> T, effect_traceback, node.cache) pop_scope() if not ok then - table.clear(update_queue) - update_queue.n = 0 - error(new_value, 0) + return false, new_value end node.cache = new_value - return cur_value ~= new_value + return cur_value ~= new_value, cleanup_err or owned_err end end +local function evaluate_node(node: Node): boolean + local changed, err = try_evaluate_node(node) + if err then error(err, 0) end + return changed +end + local function queue_children_for_update(node: SourceNode) local i = update_queue.n while node[1] do @@ -216,21 +239,30 @@ local function get_update_queue_length() return update_queue.n end +-- queued nodes are already unparented, so every one must be evaluated even if +-- another errors; the first error is thrown after the queue is flushed local function flush_update_queue(from: number) + local first_err: string? + local i = from + 1 while i <= update_queue.n do local node = update_queue[i] --assert(node.effect) - if node.owner and evaluate_node(node) then - queue_children_for_update(node) + -- check if node is still owned in case destroyed after queued + if node.owner then + local changed, err = try_evaluate_node(node) + if changed then queue_children_for_update(node) end + first_err = first_err or err end - update_queue[i] = false :: any + update_queue[i] = false :: any -- false instead of nil to avoid sparse i += 1 end update_queue.n = from + + if first_err then error(first_err, 0) end end local function update_descendants(root: SourceNode) @@ -239,21 +271,7 @@ local function update_descendants(root: SourceNode) if flags.batch then return end - local i = n0 + 1 - while i <= update_queue.n do - local node = update_queue[i] - --assert(node.effect) - - -- check if node is still owned in case destroyed after queued - if node.owner and evaluate_node(node) then - queue_children_for_update(node) - end - - update_queue[i] = false :: any -- false instead of nil to avoid sparse - i += 1 - end - - update_queue.n = n0 + flush_update_queue(n0) end local function push_scope_as_child_of(node: SourceNode) diff --git a/test/tests.luau b/test/tests.luau index cd9cd1d..dd07517 100644 --- a/test/tests.luau +++ b/test/tests.luau @@ -633,6 +633,25 @@ TEST("effect()", wrap_root(function() CHECK(count == 2) end + + do CASE "error does not stop other effects" + local src = source(0) + + local count = 0 + effect(function() + if src() == 1 then error("effect error") end + end) + effect(function() + src() + count += 1 + end) + + CHECK(count == 1) + CHECK(not pcall(src, 1)) + CHECK(count == 2) + src(2) + CHECK(count == 3) + end end)) TEST("cleanup()", wrap_root(function() @@ -709,6 +728,46 @@ TEST("cleanup()", wrap_root(function() CHECK(ran.destroy) CHECK(ran.Destroy) end + + do CASE "error does not stop destruction" + local src = source(0) + + local cleaned = false + local count = 0 + + local destroy = root(function() + cleanup(function() error("cleanup error") end) + cleanup(function() cleaned = true end) + effect(function() + src() + count += 1 + end) + end) + + CHECK(not pcall(destroy)) + CHECK(cleaned) + src(1) + CHECK(count == 1) + end + + do CASE "error on rerun does not stop updates" + local src = source(0) + + local count = 0 + effect(function() + src() + cleanup(function() if src() == 1 then error("cleanup error") end end) + end) + effect(function() + src() + count += 1 + end) + + CHECK(not pcall(src, 1)) + CHECK(count == 2) + src(2) + CHECK(count == 3) + end end)) TEST("create()", wrap_root(function() @@ -3112,6 +3171,21 @@ TEST("strict", wrap_root(function() CHECK(count == 4) end + do CASE "effect updating its own source" + local src = source(0) + local count = 0 + + effect(function() + count += 1 + if count > 100 then return end + local v = src() + if v > 0 then src(v + 1) end + end) + + CHECK(not pcall(src, 1)) + CHECK(count < 100) + end + do CASE "destruction of active scope" local src = source(false) local count = 0