Invalid events should raise rather than failing silently?

[lucasmazza]

Currently, if we call an event on the workflow object that doesn't exists in the current state, we get a no-op false return value rather than a NotMethodError or something similar. Could it be more handy for the development environment to raise an exception here to help track down bad code paths (a caller that is trying to do a wrong transition on a specific scenario) instead? Even if the client code checks for falsy return values there is no way to know that an invalid transition/event was called.

Here is a piece of code to illustrate the scenario:

class PublishingWorkflow
  include FlowMachine::Workflow

  state DraftState
  state PublishedState
end

class DraftState < FlowMachine::WorkflowState
  event :publish do
    transition to: :published
  end
end

class PublishedState < FlowMachine::WorkflowState
  event :retract do
    transition to: :retracted
  end
end

workflow = PublishingWorkflow.new(BlogPost.new(state: :draft))
# drafts can't be retracted.
workflow.retract! #=> false

[jhanggi]

That's a really good point. There should definitely be a way to tell why it didn't transition.

I'm torn between raising a NoMethodError or moving a little more logic into the may_xxx? methods and treat the respond_to? as a kind of guard method, so the method would still return false, but you could call workflow.guard_errors to get something like [:invalid_transition]. Part of the question comes from deciding whether calling an invalid transition should be considered a bug (which would suggest raising error) or more of a user-side concern.

I don't have my setup handy right now, so forgive my forgetfulness of the inner workings. In your example, after workflow = PublishingWorkflow.new(BlogPost.new(state: :draft)), what does workflow.may_retract? do? I would expect it to return false and add something to guard_errors, but it looks like it might not.

Thanks for the feedback. I'll look into this further, likely later tonight or tomorrow night.

[lucasmazza]

@jhanggi workflow.may_retract? has the same behaviour as it is proxied to the state class in the same way as the transition methods: it returns false and doesn't push anything to the guard errors Array.

[jhanggi]

Closed by PR #11.