Do not rollback errors

[dphfox]

Right now, if a Computed runs into an uncaught error, it falls back to returning the last valid value it was able to calculate. While this technically keeps the program running, it can very quickly lead to a domino effect of issues because in doing this, one of Fusion's core principles is violated.

Fusion guarantees that, at any time, the reactive graph is completely internally consistent, and that - at least conceptually - calculations always see values sampled from the same point in time. The rollback behaviour violates this principle because it allows stale values and up-to-date values to mix in user code.

There's not a clean way of signalling an error has occurred in state object code, because the Luau types don't provision for returning error codes. While this could possibly be done, it would massively bloat almost all Fusion code with error checking - even infallible code - so I don't consider it to be a viable option.

So, as an alternative, I would propose that state objects should propagate uncaught errors to any other objects attempting to use them. From there, standard error catching mechanisms like Safe can be used to deal with the error.

[dphfox]

For a concrete example of what this would do:

local badCode = scope:Computed(function()
    error("Oh no")
end)

local foo = peek(badCode) --> Error in callback: Oh no (ID: callbackError)

[dphfox]

This would significantly reduce the use of logErrorNonFatal, which is notorious for being hard to handle due to being thrown in another thread for the sole purpose of not unwinding user code. This makes it impossible to catch by the user because it doesn't invoke user-defined error handlers. Instead, this can only be detected by watching the output for errors.

[dphfox]

A consideration here is that we need to be able to store information about the original location of the error, even if errors are now being thrown from a different location in the code base.

Luckily Fusion has the parseError handler which captures a lot of information about the stack trace of an error, so it should be possible to forward location information onto the user regardless of where the error occurs.

[dphfox]

This also ties into developer experience around lazy execution, specifically around how intuitive it is to throw errors at different points in the execution of the code.

Under eager execution, errors in state objects are surfaced to users immediately after the change that causes the error. This is because the error is thrown as part of the update process, which is eager.

However, under lazy execution, the update process may happen at a completely different point in time, detaching the error temporally from its cause in ways that are hard to predict by end users.

So, instead of throwing the error from inside of the update process (which then has to be non-fatal because the update process is infallible), it's likely a better idea to throw the error when using an object in an invalid state instead. This makes the timing of errors predictable - if there's an error, it is always reliably on use.

It also presents an opportunity to allow for intermediate error states without logging errors. Consider the following reasonable code:

local items = scope:Value({"apple", "banana", "pear", "mango", "pomegranate"})
local index = scope:Value(4)

local currentItem = scope:Computed(function(use)
    local item = use(items)[use(index)]
    assert(item ~= nil, "Index is out of bounds")
    return item
end)

items:set({"apple", "mango"})
index:set(2)

This would log an Index is out of bounds error to the console, because between items:set() and index:set(), the index indeed is out of bounds. However, that intermediate value is never requested by the outside world, so the log is only noise.

The only time it's actually an error is when the intermediate value becomes visible to the outside world:

-- ... snip ...
items:set({"apple", "mango"})
print("The current item is:", peek(currentItem)) -- This is not valid!
index:set(2)

[dphfox]

An important consequence of this change would be that peek() becomes a fallible function. This makes the implementation of many state objects fallible immediately, which would be dangerous to the update process which is meant to be infallible.

Either errors would need to be caught inside the state objects, or the update process itself would need to be shielded from uncaught errors.

[dphfox]

I'm not sure how this will pan out, so here's my plan.

I'm going to implement this but gate it behind an internal constant. I'll try having it switched on for a bit, but if it causes anyone any problems, it'll be easy to turn off again.