Raise error when guard fails

[choubacha]

I'm not sure this falls inline with your design but I was expecting a guard method to raise an exception when it failed, much like invalid transitions raise errors.

Personally, I don't like a mixture of functionality, either fail hard or use the return value. With guards I need to check the return value (I think) and with invalid state transitions I need to rescue an exception.

Since guards can be multiple response, an exception might be the best solution since it can carry with it which guards failed. This is especially useful for sharing failure state with the end user.

[choubacha]

I suppose I can work around this by raising an exception in the guard. That way I know what failed but it seems hacky. Even the event_failed callback doesn't seem to get passed enough context to know why it failed.

[troessner]

I agree with you. Guards should raise just like invalid transitions.

I'll put this on my xmas list ;)

[troessner]

I did give this some thoughts and I'm not so sure anymore that this would be the right thing to do.

First of all, this:

but I was expecting a guard method to raise an exception when it failed, much like invalid transitions raise errors.

is not really true - transitions merely re-raises an exception that ActiveRecord would have thrown anyway as you can see here, so using exceptions is not something that is common in transitions.

Secondly I'm concerned what this would do to readability and maintenance. Just imagine you have 6 transitions and 3 of them have guards. In this case you'd have to:

1.) Treat 3 transitions conceptually as potentially exceptional, the other 3 not which feels weird.

2.) Treat those 3 different in source code. Meaning 3 out of 6 might have rescue blocks while the others don't. I can imagine this being confusing. And in case you remove a guard you have to potentially adapt all places where this transition is used as well.

3.) You still can make guards exceptional in your user code which keeps it out of transitions - so basically the way you do it right now. I wouldn't actually call this a workaround what you're doing, but rather a legit way.

What do you think?

[choubacha]

@troessner

Thanks for your careful thought. I think that it makes sense. But I think a small amount more documentation in the README might help clarify how to make use of a guard. Maybe even example code showing the boolean response from the event.

I'm currently raising a specific exception from within the guard which I think rolls everything back so that the state transition never happens. This allows me to handle this state transition failure in a specific way. I'm not positive that this would decrease legibility if it were to be a transitions exception though. Here's what I'm thinking it might look like:

class Model < ActiveRecord::Base

  include ActiveModel::Transitions
  state_machine do
    state :first
    state :second

    event :promote, from: :first, to: :second, guard: :will_fail
  end

  def will_fail
    false
  end

  def tester
    self.promote!
  rescue Transitions::GuardedTransition => e
    puts e.guards # => [:will_fail]
  end
end

Another possibility would be to allow me to define a hash of guards with exception classes, maybe something like:

event :promote, from: :first, to: :second, guard: { will_fail: MyOwnErrorClass }

I guess that would be a bigger change but it would allow for flexibility 😄 either way, if raising an error from within the guard is a good way to do it, i'll just do that.

[troessner]

But I think a small amount more documentation in the README might help clarify how to make use of a guard.

Excellent point. Let me think about a good example, then I'll create a pull request for our README and you can review if the example would have helped before you started to wonder why guards work the way they work :)

[troessner]

Merged #156 - closing this one.