diff --git a/lib/floe.rb b/lib/floe.rb index 41dd33fb..bef087fe 100644 --- a/lib/floe.rb +++ b/lib/floe.rb @@ -43,6 +43,8 @@ module Floe class Error < StandardError; end class InvalidWorkflowError < Error; end class InvalidExecutionInput < Error; end + class PathError < Error; end + class ExecutionError < Error; end def self.logger @logger ||= NullLogger.new diff --git a/lib/floe/validation_mixin.rb b/lib/floe/validation_mixin.rb index 33d5c184..04d7bddd 100644 --- a/lib/floe/validation_mixin.rb +++ b/lib/floe/validation_mixin.rb @@ -7,7 +7,7 @@ def self.included(base) end def parser_error!(comment) - self.class.parser_error!(name, comment) + raise Floe::InvalidWorkflowError, "#{self.class.location_name_str(name)} #{comment}" end def missing_field_error!(field_name) @@ -18,6 +18,16 @@ def invalid_field_error!(field_name, field_value = nil, comment = nil) self.class.invalid_field_error!(name, field_name, field_value, comment) end + def runtime_field_error!(field_name, field_value, comment) + raise Floe::ExecutionError, self.class.field_error_text(name, field_name, field_value, comment) + end + + def wrap_runtime_error(field_name, field_value) + yield + rescue Floe::PathError => e + runtime_field_error!(field_name, field_value, e.message) + end + def workflow_state?(field_value, workflow) workflow.payload["States"] ? workflow.payload["States"].include?(field_value) : true end @@ -29,20 +39,25 @@ def wrap_parser_error(field_name, field_value) end module ClassMethods - def parser_error!(name, comment) - name = name.join(".") if name.kind_of?(Array) - raise Floe::InvalidWorkflowError, "#{name} #{comment}" + def location_name_str(name) + name.kind_of?(Array) ? name.join(".") : name end + # these are here for State.build! + def missing_field_error!(name, field_name) - parser_error!(name, "does not have required field \"#{field_name}\"") + raise Floe::InvalidWorkflowError, "#{location_name_str(name)} does not have required field \"#{field_name}\"" end def invalid_field_error!(name, field_name, field_value, comment) + raise Floe::InvalidWorkflowError, field_error_text(name, field_name, field_value, comment) + end + + def field_error_text(name, field_name, field_value, comment = nil) # instead of displaying a large hash or array, just displaying the word Hash or Array field_value = field_value.class if field_value.kind_of?(Hash) || field_value.kind_of?(Array) - parser_error!(name, "field \"#{field_name}\"#{" value \"#{field_value}\"" unless field_value.nil?} #{comment}") + "#{location_name_str(name)} field \"#{field_name}\"#{" value \"#{field_value}\"" unless field_value.nil?} #{comment}" end end end diff --git a/lib/floe/workflow/choice_rule/data.rb b/lib/floe/workflow/choice_rule/data.rb index fb626fd5..1407218e 100644 --- a/lib/floe/workflow/choice_rule/data.rb +++ b/lib/floe/workflow/choice_rule/data.rb @@ -5,14 +5,13 @@ class Workflow class ChoiceRule class Data < Floe::Workflow::ChoiceRule def true?(context, input) + return presence_check(context, input) if compare_key == "IsPresent" + lhs = variable_value(context, input) rhs = compare_value(context, input) - validate!(lhs) - case compare_key when "IsNull" then is_null?(lhs) - when "IsPresent" then is_present?(lhs) when "IsNumeric" then is_numeric?(lhs) when "IsString" then is_string?(lhs) when "IsBoolean" then is_boolean?(lhs) @@ -47,8 +46,17 @@ def true?(context, input) private - def validate!(value) - raise "No such variable [#{variable}]" if value.nil? && !%w[IsNull IsPresent].include?(compare_key) + def presence_check(context, input) + rhs = compare_value(context, input) + # don't need the value, just need to see if the path finds the value + variable_value(context, input) + + # path found the variable_value, (so if they said true, return true) + rhs + rescue Floe::ExecutionError + # variable_value (path) threw an error + # it was not found (so if they said false, return true) + !rhs end def is_null?(value) # rubocop:disable Naming/PredicateName diff --git a/lib/floe/workflow/path.rb b/lib/floe/workflow/path.rb index fe8a3cf7..6ae06b21 100644 --- a/lib/floe/workflow/path.rb +++ b/lib/floe/workflow/path.rb @@ -34,7 +34,14 @@ def value(context, input = {}) return obj if path == "$" results = JsonPath.on(obj, path) - results.count < 2 ? results.first : results + case results.count + when 0 + raise Floe::PathError, "references an invalid value" + when 1 + results.first + else + results + end end def to_s diff --git a/lib/floe/workflow/state.rb b/lib/floe/workflow/state.rb index be096f03..0ab743b8 100644 --- a/lib/floe/workflow/state.rb +++ b/lib/floe/workflow/state.rb @@ -45,20 +45,35 @@ def wait(context, timeout: nil) # @return for incomplete Errno::EAGAIN, for completed 0 def run_nonblock!(context) start(context) unless context.state_started? + return Errno::EAGAIN unless ready?(context) finish(context) + rescue Floe::ExecutionError => e + # input / output paths were bad + context.next_state = nil + context.output = {"Error" => "States.RuntimeError", "Cause" => e.message} + # finish.super was never called + mark_finished(context) end def start(context) + mark_started(context) + end + + def finish(context) + mark_finished(context) + end + + def mark_started(context) context.state["EnteredTime"] = Time.now.utc.iso8601 logger.info("Running state: [#{long_name}] with input [#{context.json_input}]...") end - def finish(context) - finished_time = Time.now.utc - entered_time = Time.parse(context.state["EnteredTime"]) + def mark_finished(context) + finished_time = Time.now.utc + entered_time = Time.parse(context.state["EnteredTime"]) context.state["FinishedTime"] ||= finished_time.iso8601 context.state["Duration"] = finished_time - entered_time diff --git a/lib/floe/workflow/states/choice.rb b/lib/floe/workflow/states/choice.rb index d2487e5f..d8037cac 100644 --- a/lib/floe/workflow/states/choice.rb +++ b/lib/floe/workflow/states/choice.rb @@ -19,12 +19,14 @@ def initialize(workflow, name, payload) end def finish(context) - input = input_path.value(context, context.input) - output = output_path.value(context, input) + input = wrap_runtime_error("InputPath", input_path.to_s) { input_path.value(context, context.input) } + output = wrap_runtime_error("OutputPath", output_path.to_s) { output_path.value(context, input) } + # For a bad path, throws an ExecutionError next_state = choices.detect { |choice| choice.true?(context, output) }&.next || default context.next_state = next_state context.output = output + super end diff --git a/lib/floe/workflow/states/fail.rb b/lib/floe/workflow/states/fail.rb index ae0f54f8..56e28160 100644 --- a/lib/floe/workflow/states/fail.rb +++ b/lib/floe/workflow/states/fail.rb @@ -4,7 +4,7 @@ module Floe class Workflow module States class Fail < Floe::Workflow::State - attr_reader :cause, :error + attr_reader :cause, :error, :cause_path, :error_path def initialize(workflow, name, payload) super @@ -21,8 +21,8 @@ def finish(context) # see https://docs.aws.amazon.com/step-functions/latest/dg/amazon-states-language-fail-state.html # https://docs.aws.amazon.com/step-functions/latest/dg/amazon-states-language-intrinsic-functions.html#asl-intrsc-func-generic context.output = { - "Error" => @error_path ? @error_path.value(context, context.input) : error, - "Cause" => @cause_path ? @cause_path.value(context, context.input) : cause + "Error" => error_path ? wrap_runtime_error("ErrorPath", error_path.to_s) { @error_path.value(context, context.input) } : error, + "Cause" => cause_path ? wrap_runtime_error("CausePath", cause_path.to_s) { @cause_path.value(context, context.input) } : cause }.compact super end diff --git a/lib/floe/workflow/states/input_output_mixin.rb b/lib/floe/workflow/states/input_output_mixin.rb index 59b3a732..b0149baa 100644 --- a/lib/floe/workflow/states/input_output_mixin.rb +++ b/lib/floe/workflow/states/input_output_mixin.rb @@ -5,25 +5,30 @@ class Workflow module States module InputOutputMixin def process_input(context) - input = input_path.value(context, context.input) - input = parameters.value(context, input) if parameters + input = wrap_runtime_error("InputPath", input_path.to_s) { input_path.value(context, context.input) } + input = wrap_runtime_error("Parameters", parameters.to_s) { parameters.value(context, input) } if parameters input end def process_output(context, results) - return context.input.dup if results.nil? - return if output_path.nil? + results = context.input.dup if results.nil? - results = result_selector.value(context, results) if @result_selector - if result_path.payload.start_with?("$.Credentials") - credentials = result_path.set(context.credentials, results)["Credentials"] + results = wrap_runtime_error("ResultSelector", @result_selector.to_s) { result_selector.value(context, results) } if @result_selector + if result_path.nil? + output = results + elsif result_path.payload.start_with?("$.Credentials") + credentials = wrap_runtime_error("ResultPath", result_path.to_s) { result_path.set(context.credentials, results)["Credentials"] } context.credentials.merge!(credentials) output = context.input.dup else - output = result_path.set(context.input.dup, results) + output = wrap_runtime_error("ResultPath", result_path.to_s) { result_path.set(context.input.dup, results) } end - output_path.value(context, output) + if output_path + wrap_runtime_error("OutputPath", output_path.to_s) { output_path.value(context, output) } + else + output + end end end end diff --git a/lib/floe/workflow/states/succeed.rb b/lib/floe/workflow/states/succeed.rb index bbbd814b..47a51d53 100644 --- a/lib/floe/workflow/states/succeed.rb +++ b/lib/floe/workflow/states/succeed.rb @@ -14,8 +14,8 @@ def initialize(workflow, name, payload) end def finish(context) - input = input_path.value(context, context.input) - context.output = output_path.value(context, input) + input = wrap_runtime_error("InputPath", input_path.to_s) { input_path.value(context, context.input) } + context.output = wrap_runtime_error("OutputPath", output_path.to_s) { output_path.value(context, input) } context.next_state = nil super diff --git a/lib/floe/workflow/states/task.rb b/lib/floe/workflow/states/task.rb index 414b8e0c..b5e94626 100644 --- a/lib/floe/workflow/states/task.rb +++ b/lib/floe/workflow/states/task.rb @@ -116,7 +116,7 @@ def catch_error!(context, error) return if catcher.nil? context.next_state = catcher.next - context.output = catcher.result_path.set(context.input, error) + context.output = wrap_runtime_error("ResultPath", result_path.to_s) { catcher.result_path.set(context.input, error) } logger.info("Running state: [#{long_name}] with input [#{context.json_input}]...CatchError - next state: [#{context.next_state}] output: [#{context.json_output}]") true diff --git a/lib/floe/workflow/states/wait.rb b/lib/floe/workflow/states/wait.rb index e22ee5d2..99d3b464 100644 --- a/lib/floe/workflow/states/wait.rb +++ b/lib/floe/workflow/states/wait.rb @@ -28,7 +28,7 @@ def initialize(workflow, name, payload) def start(context) super - input = input_path.value(context, context.input) + input = wrap_runtime_error("InputPath", input_path.to_s) { input_path.value(context, context.input) } wait_until!( context, @@ -38,8 +38,8 @@ def start(context) end def finish(context) - input = input_path.value(context, context.input) - context.output = output_path.value(context, input) + input = wrap_runtime_error("InputPath", input_path.to_s) { input_path.value(context, context.input) } + context.output = wrap_runtime_error("OutputPath", input_path.to_s) { output_path.value(context, input) } super end diff --git a/spec/workflow/choice_rule_spec.rb b/spec/workflow/choice_rule_spec.rb index a2a9371f..c9dbde2c 100644 --- a/spec/workflow/choice_rule_spec.rb +++ b/spec/workflow/choice_rule_spec.rb @@ -93,7 +93,7 @@ let(:input) { {} } it "raises an exception" do - expect { subject }.to raise_exception(RuntimeError, "No such variable [$.foo]") + expect { subject }.to raise_exception(Floe::PathError, "references an invalid value") end end @@ -118,22 +118,51 @@ end context "with IsPresent" do - let(:payload) { {"Variable" => "$.foo", "IsPresent" => true, "Next" => "FirstMatchState"} } + let(:positive) { true } + let(:payload) { {"Variable" => "$.foo", "IsPresent" => positive, "Next" => "FirstMatchState"} } context "with null" do let(:input) { {"foo" => nil} } + it { expect(subject).to eq(true) } + end - it "returns false" do - expect(subject).to eq(false) - end + context "with false" do + let(:input) { {"foo" => "bar"} } + it { expect(subject).to eq(true) } end - context "with non-null" do + context "with string" do + let(:input) { {"foo" => false} } + it { expect(subject).to eq(true) } + end + + context "with missing value" do + let(:input) { {} } + it { expect { subject }.to raise_error(Floe::PathError, "references an invalid value") } + end + + context "with null" do + let(:positive) { false } + let(:input) { {"foo" => nil} } + it { expect(subject).to eq(false) } + end + + context "with false" do + let(:positive) { false } let(:input) { {"foo" => "bar"} } + it { expect(subject).to eq(false) } + end - it "returns true" do - expect(subject).to eq(true) - end + context "with string" do + let(:positive) { false } + let(:input) { {"foo" => false} } + it { expect(subject).to eq(false) } + end + + context "with missing value" do + let(:positive) { false } + let(:input) { {} } + it { expect { subject }.to raise_error(Floe::PathError, "references an invalid value") } end end @@ -279,6 +308,11 @@ expect(subject).to eq(false) end end + + context "with path not found" do + let(:input) { {"foo" => 2} } + it { expect { subject }.to raise_error(Floe::PathError, "references an invalid value") } + end end context "with a NumericLessThan" do diff --git a/spec/workflow/intrinsic_function_spec.rb b/spec/workflow/intrinsic_function_spec.rb index 17510038..14a31fb4 100644 --- a/spec/workflow/intrinsic_function_spec.rb +++ b/spec/workflow/intrinsic_function_spec.rb @@ -655,8 +655,8 @@ end it "handles invalid path references" do - result = described_class.value("States.Array($.xxx)", {"context" => {"baz" => "qux"}}, {"input" => {"foo" => "bar"}}) - expect(result).to eq([nil]) + ctx = {"context" => {"baz" => "qux"}}, {"input" => {"foo" => "bar"}} + expect { described_class.value("States.Array($.xxx)", ctx) }.to raise_error(Floe::PathError, "references an invalid value") end end diff --git a/spec/workflow/path_spec.rb b/spec/workflow/path_spec.rb index d2c52d8d..f405fa4b 100644 --- a/spec/workflow/path_spec.rb +++ b/spec/workflow/path_spec.rb @@ -11,15 +11,15 @@ describe "#value" do context "referencing the global context" do it "with a missing value" do - expect(described_class.new("$$.foo").value({})).to be_nil + expect { described_class.new("$$.foo").value({}, {"foo" => "bar"}) }.to raise_error(Floe::PathError, "references an invalid value") end it "with a single value" do - expect(described_class.new("$$.foo").value({"foo" => "bar"})).to eq("bar") + expect(described_class.new("$$.foo").value({"foo" => "bar"}, {})).to eq("bar") end it "with a nested hash" do - expect(described_class.new("$$.foo.bar").value({"foo" => {"bar" => "baz"}})).to eq("baz") + expect(described_class.new("$$.foo.bar").value({"foo" => {"bar" => "baz"}}, {})).to eq("baz") end it "with an array" do @@ -33,7 +33,7 @@ context "referencing the inputs" do it "with a missing value" do - expect(described_class.new("$.foo").value({"foo" => "bar"})).to be_nil + expect { described_class.new("$.foo").value({"foo" => "bar"}, {}) }.to raise_error(Floe::PathError, "references an invalid value") end it "with a single value" do diff --git a/spec/workflow/state_spec.rb b/spec/workflow/state_spec.rb index 850a3986..2c4943a9 100644 --- a/spec/workflow/state_spec.rb +++ b/spec/workflow/state_spec.rb @@ -45,7 +45,7 @@ it "is finished" do state.start(ctx) - state.finish(ctx) + state.mark_finished(ctx) expect(ctx.state_started?).to eq(true) end @@ -63,7 +63,7 @@ it "is finished" do state.start(ctx) - state.finish(ctx) + state.mark_finished(ctx) expect(ctx.state_finished?).to eq(true) end diff --git a/spec/workflow/states/choice_spec.rb b/spec/workflow/states/choice_spec.rb index 754fe392..b62bd2a7 100644 --- a/spec/workflow/states/choice_spec.rb +++ b/spec/workflow/states/choice_spec.rb @@ -49,9 +49,7 @@ describe "#run_nonblock!" do context "with a missing variable" do - it "raises an exception" do - expect { state.run_nonblock!(ctx) }.to raise_error(RuntimeError, "No such variable [$.foo]") - end + it { expect { workflow.run_nonblock }.to raise_error(Floe::PathError, "references an invalid value") } end context "with an input value matching a condition" do diff --git a/spec/workflow/states/pass_spec.rb b/spec/workflow/states/pass_spec.rb index 112fa9ae..5eacadd5 100644 --- a/spec/workflow/states/pass_spec.rb +++ b/spec/workflow/states/pass_spec.rb @@ -138,6 +138,28 @@ end end + context "With a missing InputPath" do + let(:payload) do + { + "PassState" => { + "Type" => "Pass", + "End" => true, + "InputPath" => "$.missing" + } + } + end + it "completes with an error" do + workflow.run_nonblock + expect(ctx.state_finished?).to eq(true) + expect(ctx.output).to eq( + { + "Cause" => "States.PassState field \"InputPath\" value \"$.missing\" references an invalid value", + "Error" => "States.RuntimeError" + } + ) + end + end + # https://states-language.net/#inputoutput-processing-examples context "with 2 blocks" do let(:payload) do @@ -196,6 +218,18 @@ end end + context "with Invalid InputPath" do + let(:input) { {} } + let(:payload) do + {"Pass" => {"Type" => "Pass", "End" => true, "InputPath" => "$.color"}} + end + + it "detects missing value" do + workflow.run_nonblock + expect(ctx.output).to eq({"Cause" => "States.Pass field \"InputPath\" value \"$.color\" references an invalid value", "Error" => "States.RuntimeError"}) + end + end + context "with OutputPath" do let(:input) { {"color" => "red", "garbage" => nil} } let(:payload) { {"Pass" => {"Type" => "Pass", "End" => true, "OutputPath" => "$.color"}} }