Error when conditional transition is undefined

[craiglittle]

Hey there!

I'm getting an exception when attempting to make a transition that is not defined in my state machine. I'm expecting it to not transition silently as stated in the docs under the "Dynamic choice conditions" section:

However if default state is not present and non of the conditions match, no transition is performed.

Here's the relevant code:

FiniteMachine.define do
  initial :inactive

  target ticket

  alias_target :ticket

  events {
    event :advance, :from => [:inactive, :paused, :fulfilled] do
      choice :active, :if => -> {
        ticket.working? && !ticket.pending? && !ticket.on_hold?
      }
    end

    event :advance, :from => [:inactive, :active, :fulfilled] do
      choice :paused, :if => -> { ticket.pending? || ticket.on_hold? }
    end

    event :advance, :from => [:inactive, :active, :paused] do
      choice :fulfilled, :if => -> { ticket.finished? }
    end
  }
end

For example, when in the paused state and calling advance while the ticket is still in the paused state, I'm getting the following exception:

FiniteMachine::TransitionError: NoMethodError: undefined method `to_states' for nil:NilClass
    occured at /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/transition.rb:202:in `update_state'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/transition.rb:232:in `block in call'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:233:in `block in sync_synchronize'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `handle_interrupt'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `sync_synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/two_phase_lock.rb:26:in `synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/threadable.rb:14:in `sync_exclusive'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/transition.rb:229:in `call'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/state_machine.rb:283:in `block in transition'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:233:in `block in sync_synchronize'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `handle_interrupt'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `sync_synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/two_phase_lock.rb:26:in `synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/threadable.rb:14:in `sync_exclusive'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/state_machine.rb:274:in `transition'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/event.rb:96:in `block in call'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:233:in `block in sync_synchronize'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `handle_interrupt'
    /opt/.rbenv/versions/2.1.6/lib/ruby/2.1.0/sync.rb:230:in `sync_synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/two_phase_lock.rb:26:in `synchronize'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/threadable.rb:14:in `sync_exclusive'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/event.rb:91:in `call'
    /home/vagrant/bundle/ruby/2.1.0/gems/finite_machine-0.10.0/lib/finite_machine/event_builder.rb:61:in `block in define_event_transition'
    (irb):3:in `irb_binding'

I'm able to avoid the exception by adding a few more conditional transitions:

event :advance, :from => :active do
  choice :active, :silent => true
end

event :advance, :from => :paused do
  choice :paused, :silent => true
end

event :advance, :from => :fulfilled do
  choice :fulfilled, :silent => true
end

However, it'd be nice if I didn't have to add this boilerplate.

Am I doing something wrong here? Please let me know if you'd like me to provide any more context.

Thanks!

[craiglittle]

Actually, even adding those boilerplate transitions still doesn't work for me.

It seems as though the gem currently doesn't handle choice pseudostates properly when there are multiple definition blocks for the same event.

For example, given the following event definition:

events {
  event :advance, :from => :inactive do
    choice :active, :if => -> {
      ticket.working? && !ticket.pending? && !ticket.on_hold?
    }

    choice :paused, :if => -> { ticket.pending? || ticket.on_hold? }

    choice :fulfilled, :if => -> { ticket.finished? }
  end

  event :advance, :from => :paused do
    choice :active, :if => -> {
      ticket.working? && !ticket.pending? && !ticket.on_hold?
    }

    choice :fulfilled, :if => -> { ticket.finished? }
  end
}

When the state machine is in the paused state and advance is called while the ticket still matches the paused conditional, I still get the TransitionError exception.

I'm going through these gymnastics because I'm trying to fire a callback only when an entirely new state is entered (i.e. not when going from active to active). If that callback existed (let's call it on_change), then the following simpler event definition would work:

events {
  event :advance, :from => :any do
    choice :active, :if => -> {
      ticket.working? && !ticket.pending? && !ticket.on_hold?
    }

    choice :paused, :if => -> { ticket.pending? || ticket.on_hold? }

    choice :fulfilled, :if => -> { ticket.finished? }
  end
}

callbacks {
  on_change :to => :paused do
    # perform action only when transitioning from any other state *besides* 'paused'
  end
}

I hope that's helpful. I'd love for the new callback to be added! Also, I'm happy to take a crack at it if you don't have the time to do it yourself (and you're interested in the feature).

[piotrmurach]

Hi Craig,

Thanks for using the library. A scenario when using choice with multiple event definitions under the same name is probably not tested and hence I consider it to be a bug. I will look into it.

However, in your case I believe that using choice similar to your last example is the right way to go about it. This is suitable to split decision where two or more possible paths can be chosen and you want to select one of them. Does that actually work for you?

I'm not sure if I fully understand your motivation for a new callback. Are the current callbacks not sufficient? For example, if I wanted to filter out certain states or ensure that only actual state change when performed is taken into account, I would query the transition event object passed to each callback:

on_transition { |event|
  if event.from != event.to && event.to == :paused
    # perform logic for actual state change to 'paused' state
  end
}

I want to keep the library API down to the most necessary and hence would be reluctant to add new methods if the behaviour can be achieved with current implementation. Let me have your thoughts!

[piotrmurach]

@craiglittle Unfortunately I cannot reproduce the issue with multiple choice statements for the same event name. I have attached a spec to confirm correct working, if you could adjust to replicate the error condition and make the spec fail then I'm happy to fix it. Otherwise I would need exact steps and implementation details for your machine that led to the error.

[craiglittle]

Unfortunately I cannot reproduce the issue with multiple choice statements for the same event name. I have attached a spec to confirm correct working, if you could adjust to replicate the error condition and make the spec fail then I'm happy to fix it. Otherwise I would need exact steps and implementation details for your machine that led to the error.

Here's a failing spec that replicates the bug: https://github.com/craiglittle/finite_machine/commit/d978300bf7f1bd08821fc7244a25d3d3330ec0a1

I think it occurs only when there are multiple choice statements with multiple possible from states where none of the choices have a matching if clause.

I'm not sure if I fully understand your motivation for a new callback. Are the current callbacks not sufficient?

You're correct that the proposed callback behavior could be produced by querying the event object in an existing callback; however, 1) I find it a bit inelegant to do so, and 2) I think there's something unique about a transition from a state to itself that would be nice to have supported at the callback API level, whereas I have a harder time finding a useful distinction between on_enter and on_transition (at first, I actually thought the latter would provide the behavior I was looking for out of the box, given the name).

Further, the behavior I'm looking for is somewhat provided by the once_on_* callbacks, albeit in a less useful way for my purposes as they fire even when trying to bring back the current state using initial, which doesn't seem right to me.

In any case, thanks for digging into the issue(s)!

[piotrmurach]

@craiglittle Could you please submit pull request with your test case? I will then look into fixing the problem. Regarding the initial, it has few quirks but underneath it is just another sugar for actual event. It transitions from start state :none to :given_state and generates init event by default read more. You can pass silent: true to silence the callbacks, please see https://github.com/peter-murach/finite_machine/issues/16 for more details of why this is the case. Please see also restore method for alternative way to setup initial state.

[craiglittle]

As requested, https://github.com/peter-murach/finite_machine/pull/36.

I should be able to handle my use case in a clean way when this bug is resolved. Like I said, I'm happy to take a crack myself if you find that you don't have the time.

[piotrmurach]

Closing issue as resolved by the pull requests #36 and #39