Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[do not merge] Allow wildcard transitions to act as "error handlers" #2935

Closed
wants to merge 2 commits into from
Closed

[do not merge] Allow wildcard transitions to act as "error handlers" #2935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddba455
Allow wildcard transitions to act as "error handlers"
Andarist Jan 11, 2022
1afe0e6
Merge remote-tracking branch 'origin/next' into andarist/wildcards-fo…
Andarist Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/core/src/StateMachine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,7 @@ export class StateMachine<
actorCtx: ActorContext<TEvent, typeof state>
): State<TContext, TEvent, TActor, TTag, TOutput, TResolvedTypesMeta> {
// TODO: handle error events in a better way
if (
isErrorActorEvent(event) &&
!state.nextEvents.some((nextEvent) => nextEvent === event.type)
) {
if (isErrorActorEvent(event) && !state.can(event as any)) {
return cloneState(state, {
error: event.data
});
Expand Down
27 changes: 27 additions & 0 deletions packages/core/test/invoke.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2803,6 +2803,33 @@ describe('invoke', () => {
});
actor.start();
});

it('a wildcard transition should be able to receive an error event', (done) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new test case that only passes with the introduced implementation change.

It initially made sense to me to introduce this change in a vein that "error transitions are just transitions". However, after thinking about it:

  • this is somewhat implicit and might not meet people's expectations
  • it hurts the types - errors in JS are already hard to type as you can throw "just anything". We could, maybe, just add a special event like { type: 'error.state', data: unknown } to the list of known events that would have to be handled by users (and that would be generated by the typegen and so on) but initially I've thought about "scoping" error event names to make the implementation of the ideas in RFC: Errors in XState rfcs#4 easier (we need to deconflict per state onError transitions, this could be done with closure-based guards but that's suboptimal for visualization and all). If we go with a solution that is based on scoping those events then it, IMHO, becomes impractical to add a, potentially huge, list of possible error event names to all wildcard transitions. OTOH wildcard transitions should be discouraged so maybe this is not that big of a problem.

TLDR. it should be decided how this should behave as part of the statelyai/rfcs#4 . Note that the first argument still stands, it's not that obvious that * would "turn off" all of the assumed "error semantics", so it might make sense to special case treatment of error.* events in this regard either way.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeaaah, I think that xstate.error.* events should be treated in a special way. Wildcard transitions should not match them.

const machine = createMachine({
initial: 'fetching',
states: {
fetching: {
invoke: {
src: fromPromise(() => Promise.reject(new Error('test err'))),
onDone: {
target: 'complete'
}
},
on: {
'*': {
actions: () => {
done();
}
}
}
},
complete: {
type: 'final'
}
}
});
createActor(machine).start();
});
});

it('invoke `src` can be used with invoke `input`', (done) => {
Expand Down