Exception from OnEntry action should not result in queued trigger being executed after next trigger

[Snaaio]

I would like to design a state machine in which an exception thrown from an OnEntry action can propagate to the caller firing the trigger. Unfortunately I saw some weird behavior regarding the queuing of triggers.

As a workaround I upgraded to Stateless v4.2.0 and configured my statemachine to use FiringMode.Immediate, but it would be nice to see the default FiringMode.Queued play a bit nicer with exceptions being thrown.

Possible options to better handle this:

1. Catch exception and execute queued triggers before rethrowing
2. Catch exception and clear queued triggers before rethrowing
3. Do not catch exception and retain queued triggers (current behavior)
   Add new method(s) on StateMachine to:
   A. Fire queued triggers
   B. Clear queued triggers

enum State { Idle, Initializing, Ready, Error }
enum Trigger { Init, Ready, Error }

[Test]
public void NUnitTestStateless_FireErrorTriggerFromOnEntryActionAndThenThrow_ShouldNotExecuteQueuedErrorTriggerAfterNextTrigger()
{
    var stm = new StateMachine<State, Trigger>(State.Idle);
    stm.Configure(State.Idle)
        .Permit(Trigger.Init, State.Initializing);
    stm.Configure(State.Initializing)
        .Permit(Trigger.Ready, State.Ready)
        .Permit(Trigger.Error, State.Error)
        .OnEntry(() =>
        {
            try
            {
                throw new InvalidOperationException("Test exception.");
            }
            catch
            {
                stm.Fire(Trigger.Error);
                throw;
            }
        });
    try
    {
        stm.Fire(Trigger.Init);
    }
    catch
    {
        // Normally the exception would be handled here
    }

    stm.Fire(Trigger.Ready); // Fails with System.InvalidOperationException : No valid leaving transitions are permitted from state 'Ready' for trigger 'Error'. Consider ignoring the trigger.

    Assert.That(stm.State, Is.EqualTo(State.Ready));
}

[HenningNT]

What was the weird behaviour, and what was the expected behaviour.

Exception handling can be hard to get right, and at the moment it is up to the user of stateless to deal with that problem. Different users will having different exception handling needs so it's tricky to cater for all possible requirements, so I'd like to leave as much as possible of the exception handling to the user.

But as a compromise we could add a OnException method where we can assign an action, very much like the OnUnhandledTrigger method. We could also provide a way of clearing the trigger queue. Maybe also add an OnError handler for each state configuration as well?

What do you think?

[Snaaio]

@HenningNT The weird behaviour was:

1. Initially a caller fires a first trigger A (from externally), which results in a state transition and execution of the corresponding OnEntry action
2. From that OnEntry action a trigger is fired, which is queued internally by stateless
3. Later in that same OnEntry action, an exception is thrown which is unhandled inside the OnEntry action and propagates through stateless to the caller of the initial trigger
4. The weird behaviour: When the caller handles the exception and fires a second trigger B (from externally), the old leftover queued triggers get executed after this second trigger.

While the current behaviour does not seem right, I am struggling with what the expected behaviour should be. After an unhandled exception I think any remaining queued triggers should either:

1. be fired on the next trigger, with all triggers being executed in their original order
2. not be fired at all
   Or maybe there is no universally correct choice here and the developer must be able to decide what happens?

[Snaaio]

@gerardog You mentioned you stopped using OnEntry actions in your comment here:
#190 (comment)

May I ask how you would design a stateless state machine when you have the following requirements:

1. Need to execute an action on entry of a state (or on activate, what is the difference?);
2. That action can throw an exception;
3. When that action throws an exception, the state machine should transition into an error state;
4. The original exception needs to be rethrown to the caller of the trigger on which the action was executed.

[gerardog]

@Snaaio, in your case I would wrap all stm.Fire() method calls inside a wrapper function, lets call it FireTrigger(). Inside it I would: (untested, just pseudocode)

public void FireTrigger(Trigger trigger)
{
    try { stm.Fire(trigger); } 
    catch (Exception ex) 
    {
        stm.Fire(Trigger.Error); 
        throw;
    }
}

This way you don't have to deal with the internal trigger queue from Stateless, which you mentioned that has issues when an exception is triggered.

[HenningNT]

I'm fixing the out-of-order trigger execution, but leaving the error handling untouched.