NoMethodError problem

[piotrmurach]

NoMethodError problem

[piotrmurach]

HI @jasonl99, thanks for using the library!

If possible at all, could you please paste in the code that causes this, that would definitely help in zeroing on the issue.

[jasonl99]

Of course! (and thank you for the library by the way)... I think the problem I'm running into is this line:

self.current_state = self.fm.current.to_s

Once I commented that out in my code, I no longer received the error.

It's been there since the beginning about version 0.50, but I'm wondering if this is bouncing back and forth between states somehow. It does look like the error is being caused by stack level too deep type situation that isn't being caught. The code below is in a "FiniteMachine" module which is included in various models.

    def fm
       context = self
      @fm ||= FiniteMachine.define do

        target(context)

        initial get_state
        terminal :completed

        events {
          event :validated, [:none, :admin_review, :invalid] => :valid
          event :validated, ready: :ready  # effectively, this is revalidating
          event :invalidated, [:admin_review, :valid] => :invalid
          event :flagged, [:valid, :ready, :final] => :admin_review
          event :readied, [:valid, :ready] => :ready
          event :finalized, [:ready,:valid,:admin_review ] => :final
          event :scored, final: :scored
          event :closed, [:admin_review, :scored ] => :complete
        }

        callbacks {
          on_transition {|event| set_state(event)}
        }

        handle FiniteMachine::TransitionError, with: proc { |exception| log "Error transitioning in state machine [#{exception.message}]", :error}

      end
    end

    def set_state(event)
      puts "Attempting to set state to #{event.to}  #{Time.now.to_i}"
      # queueing will probably happen here, but for now, just call the stuff directly
      log "Setting entity state to #{event.to} ", :info
        case event.to
        when :valid
          log "Entity has been validated", :debug
          # context.create_timeframes
        when :ready
          log 'Entity has been readied', :debug
        when :admin_review
          log "Entity has been flagged and set to notify admin", :error
          # notify admin, adjust timeframes to be unavailable, etc.
        when :final
          log 'Entity finalized',:debug
          # run context.score_results
          # self.scored
        else
        end
        # self.current_state = self.fm.current.to_s
    end

    def get_state
       self.current_state
    end

  end

[jasonl99]

I haven't looked at my finite_machine code for a good couple of months, and I'm pretty sure something is just ping-ponging the state back and forth (I'm almost certain it's related to how the states are set vs how they were set in version 0.50). I'll mark this as closed for now and I'm going to redo my states and see what happens.

[jasonl99]

Just a quick note: I think I found part of the issue.
I have a model that contains a finite_machine variable (see above @fm ||= FiniteMachine.define)

If I call this: myobject.fm.blah
and #blah is not a defined transition event, finite_machine calls myobj.blah directly. That doesn't seem to be a good thing. In other words, @fm should only attempt to call its own events.

[jasonl99]

#method_missing in the StateMachine module is the cause for me (and it was my own bug that exposed the problem). If target responds to a method, it is called regardless of whether or not it is a defined event. This was a problem in my code because I had a 'finalized' event in finite_machine, and a 'finalize' method that was used to apply the finalized state. I accidentally called fm.finalize (instead of fm.finalized) which meant that the state never changed, but the finalization code kept being called.

I suggest you raise an error in #method_missing if a call is made to a method that is not a defined event.

Either that, or I am completely misunderstanding how to use this gem :)

[piotrmurach]

This behaviour is intended, that is, when you call, for instance, methods inside the callback they are called on target context if they are not events themeselves. However I agree that calling a target object method outside of finite machine definition should probably raise the error. I suppose this would be harder to implement as I currently do not store the context of execution and verify that it happens in the block scope of, for example, callback definition.

[jasonl99]

I'm not sure it needs to be complicated...

In the method_messing method

def method_missing(method_name, *args, &block)
      if target.respond_to?(method_name.to_sym)
        target.public_send(method_name.to_sym, *args, &block)
      elsif observer.respond_to?(method_name.to_sym)
        observer.public_send(method_name.to_sym, *args, &block)
      else
        super
      end
    end

If you just changed

if target.respond_to?(method_name.to_sym)

to

if target.respond_to?(method_name.to_sym) && self.transitions.keys.include?(method_name)

I think that would be enough, right?

[piotrmurach]

I'm don't think your solution addresses the problem. Let's say you have as your target ActiveRecord instance called car. Further, let's say we want to call save method inside our callback like so:

on_enter :green do 
  save
end

With the proposed solution this simply wouldn't work as the save is not an event. This simply breaks delegation. To tackle your case we need to distinguish when we call the method. So if we do

car.fsm.save # => this should raise error as we don't want to execute target methods on state machine

However, if you are inside the callback save should be allowed. That's why the code would have to be changed to be aware when the call happens and if the error is appropriate. And if you don't want the state machine to be aware of the context, you don't need to pass the target at all. This will always raise an error if the method is not owned by state machine. Context is essential in your case for set_state to work otherwise your solution would break that.

[jasonl99]

In your example, if you simply added context, would that work?

on_enter :green do 
  context.save
end

I'd be a little worried about finite machine's auto-created method names hijacking the names of the context's methods. For example, if I have a finite machine that targets an ActiveRecord object, and a finite machine event #save which one is called? The finite machine save or the context.save? What about #save! (if both had a #save! method). By requiring the target object (context.save) the namespace automatically avoids clashes.

I think this discussion helps me understand the intent of the #target method.

[piotrmurach]

Agree. You should be able to do it now but instead of context call target in your callback see .

[piotrmurach]

@jasonl99 I have removed the implicit context when the state machine is associated with another object through target helper. So now, you need to specifically call target inside the callback to reference external methods

on_enter :green do |event|
  target.save  # => executes save on target object
  save # => executes event called save or raises error 'method undefined'
end

As you rightly pointed out this will simplify interaction with the FiniteMachine in the future and remove unambiguous calls. In other words, state machine will only be able to call its own methods unless told explicitly to call methods on target. This is part of new 0.9.0 release.