[WIP] Abort if StartAt or Next [state] is not valid #125
Conversation
I was running a workflow with an invalid StartAt
This caused a strange error
before
------
```
lib/floe/workflow.rb:76:in `step_nonblock': undefined method `run_nonblock!' for nil:NilClass (NoMethodError)
current_state.run_nonblock!
^^^^^^^^^^^^^^
from lib/floe/workflow.rb:63:in `step'
from lib/floe/workflow.rb:58:in `run!'
from exe/floe:37:in `<top (required)>'
```
after
-----
```
{"Error"=>"States.Runtime", "Cause"=>"State \"FirstState\" does not exist"}
```
kbrock
changed the title
|
@kbrock this seems like something we should catch at load time, at least StartAt and static Next states (maybe even validate all choice state targets are valid states) |
kbrock
changed the title
|
keep the spec. |
|
Checked commits kbrock/floe@1eb25f2~...6cdcf85 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint |
kbrock
force-pushed
the
check_state_name
branch
from
6cdcf85
to
1eb25f2
Compare
|
TODO: rail at load time rather than catching this exception. I still feel we need better runtime failures, but agree this error in particular needs to happen at load/validation time |
|
If we add validation on |
| context.state = {"Name" => context.next_state, "Input" => context.output} if context.next_state | ||
| desired_state = context.next_state || context.state_name | ||
| if !@states_by_name.key?(desired_state) | ||
| context.state["Error"] = "States.Runtime" |
There was a problem hiding this comment.
Is this actually a runtime error? It seems like more of a workflow definition error.
There was a problem hiding this comment.
We had a task/issue somewhere to validate workflows before we run them and raise exceptions early. I'm wondering if now is the time to add in that logic, and then have a separate Workflow#validate or Workflow#valid? method with this as the first thing we check for. Thoughts?
There was a problem hiding this comment.
WIP: make this a payload parsing check
That's maybe similar to what I'm saying, but it's not a parse failure and more of a logical validation failure. I think of parsing failures more of like being bad syntax. Perhaps I'm over-engineering though?
|
Going with validation instead: #130 |
I tried to run a workflow with an invalid StartAt
This caused a strange error
before
after