-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: Errors in XState #4
base: main
Are you sure you want to change the base?
Conversation
Overall, I'm very much in favor of immediately terminating when an unhandled exception occurs. It is by far the safest approach, instead of leaving the system in an indeterminate state. My two cents on some specifis:
100% agree that the program should terminate on unhandled errors
Super agree with recursive crashing until some parent actor handles the error, or crash the whole thing
The raise('EV') example is confusing - I personally had forgotten what raise() does, and probably a lot of people without deep familiarity with XState won't know what it does - It almost looks like it's trying to raise an exception! Does send('EV', { to: 'actor'}) do something similar, for the purposes of the example?
This is good to have but should be an extra API on top of being able to raise normal errors of any type. |
With this section, I agree with the problem (that the next state is settled before executing actions which may throw) but I don't fully agree with the solution (execute actions while determining the next state). Here's how I think it should go: An example of this behavior, given this example machine: createMachine({
initial: 'A',
states: {
A: {
on: { EVENT: { target: 'B', actions: ['action1', 'throws', 'action3'] } },
onError: 'X'
},
B: {},
X: {}
}
});
The way we can implement this is by inverting the order of updating the state in the interpreter; instead of settling on the state and then executing actions, we determine what the next state should be (before settling on it), execute actions, and then settle. Between executing actions and settling, an error may occur which may raise an event, and the new potential state will be determined based on the previously settled state and the raised event (eg., an |
I may be completely on the wrong train of thought, but this only seems to work for exit and transition actions, as you are effectively still in the process of leaving the previous state. However for entry actions, aren't you effectively in the new state at that point in time? This leads me to think that if Though as I write this, there is the possibility of performing multiple exit, transtion, enter actions for different states... Any interim states between current and next. So that would then mean an action needs an associated |
Hi @Andarist, Here are my thoughts on the RFC: I agree with your criticism about the spec. The default behavior is to swallow JS errors, which is an easy gotcha, and not very friendly to new users. It puts the responsibility on the developer to explicitly watch for these errors, which is not a great design choice. At the same time, I think catching errors and emitting them back into the state machine is a good design choice, because then you can handle the errors in your state machine logic, which is a useful thing to do. A common pattern I have implemented in SCXML is to create a top-level Error state, a top-level transition on "error.*", with generic error handling behavior that operates on _event.data (which is populated with a JavaScript Error object). You want to potentially capture this "recoverable error" state, as you want to respond to events with different behavior based on being in this state. Finally, letting the application crash on error is not a bad idea. This reminds me of Erlang's philosophy, which is to just let the program crash and resurrect itself into a good state. Therefore, the approach I like best with regard to handling errors in actions, is to:
Thank you for your time and attention to this. |
That's exactly my line of thinking 👍
In v5 we have introduced "partial" descriptors, so for instance
No, this example is specifically illustrating a problem with internal events. Internal events (raised ones) are basically always resolved first. Imagine that XState maintains 2 message queues: internalQueue: [],
externalQueue: [], and events can be sent to a particular machine at any time. We always "drain" the
Agreed, unhandled errors might always happen and we must be able to capture and process them.
I don't think this would play out well in practice. Consider this example: createMachine({
initial: 'A',
states: {
A: {
on: { EVENT: 'B },
onError: 'X'
},
B: {
entry: ['action1', 'throws', 'action3'],
onError: 'Y'
},
X: {},
Y: {},
}
}); What would you expect, as the reader, of this? Would we get to X or Y state? IMHO the reasonable approach is to end up in I think it's very important to allow for "local" reasoning. If I take a look at any particular action I should be able to only check state node on which this action is defined (and its ancestors) for Wehn I think about I see states being wrapped with try/catches and it builds a familiar and nice mental model for this: try {
try {
} catch (err) {
}
} catch (err) {
} Notice how this structure resembles the state tree! It's also hierarchical - so if I imagine this imaginary thing then it just "fits" nicely together:
By looking at this I can always expect going into
@nobrayner I believe that you are agreeing here with me and with the example that I've just put here, in this comment, right? Could you confirm that we are on the same page? Or maybe I've misunderstood you.
If we consider what "state" is in XState then I would classify this as slightly incorrect approach. I propose to use "state nodes" as the base for createMachine({
initial: 'A',
states: {
A: {
on: { EVENT: 'B },
},
B: {
entry: ['action1', 'throws', 'action3'],
onError: 'X'
states: {
C: {
onError: 'Y'
}
}
},
X: {},
Y: {},
}
}); I think that in this exaample, it makes sense to transition to
Yes, this :P (sorry, I just go through all comments top-down and respond bit by bit). So I guess we are also on the same page after all, when it comes to the example above.
Yep, this reflects my tran of thought. I want to make users to explicitly implement those top-level transitions for
I won't lie - Erlang's philosophy was a good validation metric for me when proposing this :)
I've mentioned in the RFC that I find it a little bit problematic that those error events in SCXML are "just" raised. This introduces risks because other actions in the current microstep (and even in next microsteps if the internalQueue isnt currently empty) to just "continue". I propose to make those raised error events of even a higher priority - so the transition would be selected based on it ASAP. On top of that I propose to skip executing entry: ['action1', 'throws', 'action3'], What are your thoughts on that?
Where it should be rethrown though? How the user is supposed to "catch" them? Should they be able to prevent machine failure from such a catch handler that would not be the part of machine's transitions? And if you agree with the "let the program crash" philosophy, then shouldnt we... crash that machine? And notify its parent (if there is any)? |
Context:
Any error that has no clear meaning in the domain should halt the state machine and propagate up to the application - halt as killed/frozen; no state changes or anything. On the server-side, most of the non-domain errors are fallacies of distributed computing. Timeouts are there; retries are needed. The core service has to guarantee the consistency of an aggregate (inside its boundaries) - so it also has to be idempotent while reacting to domain events and such. (IMHO) Those are not the concerns that should be covered by The way I see Unless it is taking a path of becoming some "cloud-native"y, all-in-one solution (I am not saying it would be bad or good. It's just a very different thing). (I could be wrong) |
@Andarist Would love some more details about how spawn works with this RFC. I'm guessing that spawned machines do this when they error:
This feels like it makes sense to me. It would be more ergonomic to add an |
We should also preserve the original error message (including stack trace) as much as possible. |
View rendered text