With HSM, getting AttributeError when I expect MachineError

[hsharrison]

Modifying the HSM example from the readme:

from transitions.extensions import HierarchicalMachine as Machine

states = ['standing', 'walking', {'name': 'caffeinated', 'children':['dithering', 'running']}]
transitions = [
  ['walk', 'standing', 'walking'],
  ['stop', 'walking', 'standing'],
  ['drink', '*', 'caffeinated'],
  ['walk', ['caffeinated', 'caffeinated_dithering'], 'caffeinated_running'],
  ['relax', 'caffeinated', 'standing']
]

machine = Machine(states=states, transitions=transitions, initial='standing')
machine.trigger('stop')

If the last line is machine.stop(), it raises MachineError. If the last line is machine.trigger('stop'), it raises AttributeError. This is due to the following try/except: https://github.com/pytransitions/transitions/blob/c275526a7f08c86659274d2d823487931ac7cb76/transitions/extensions/nesting.py#L851-L852

What is the logic of this? It caused some confusing behavior for me as I was merging several models by catching AttributeError and trying the trigger on the next model if it didn't exist in the previous one. Maybe there's a better way to do this, but still, I would expect a MachineError here.

[aleneum]

Hello @hsharrison,

I would expect a MachineError here.

I agree, this appears to be inconsistent. AttributeError should be raised when a transition does not exist (e.g. model.error() and model.trigger('error')). MachineError should be raised when the transition exists but is not valid (e.g, model.stop() and model.trigger('stop')). Basically the scheme recommended in #180. I need to review some tests that (falsely) expect AttributeError when MachineError would be more appropriate.

[aleneum]

This might take a bite longer since the recursive call HierarchicalMachine._trigger_event returns:

1. True when a transition was successful
2. False when a valid transition was found but conditions failed
3. None when the event does not exist OR the found transitions are not valid from the current state

The ambiguity in 3) needs to be resolved to raise the correct exception. (Re-)raising AttributeError might be an option. However, since not handling an event is perfectly fine in a (nested) state, EAFP might not be the right pattern here.

[aleneum]

With 3221eec, HierarchicalMachine._check_event_result now checks whether a trigger is known when HierarchicalMachine._trigger_event returned None. It will raise a MachineError when HierarchicalMachine.has_trigger returns True and an AttributeError otherwise. Note that no exception is raised when ignore_invalid_triggers has been set, either on the machine level or on the current state level. I hope this solves your issue. Please comment if this hasn't been solved so I can reopen the issue.

[hsharrison]

Thanks, that was quick! I did eventually come to the same conclusion as you, that EAFP wasn't the best pattern here, but in any case I think your solution makes sense. I'll let you know if I change my mind when I see it in action ;)

[aleneum]

I did eventually come to the same conclusion as you, that EAFP wasn't the best pattern here

well, I am pretty glad that you gave it a try though to spot that bug. I hope I find a way to resolve the ambiguity of 3) while processing events without the need for an explicit trigger check. But I guess its better to be consistent than inconsistent and slightly faster.