From cf60580e1f4659cd9d05857f8a36b344a930edf2 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Thu, 11 Jul 2024 18:46:37 -0400 Subject: [PATCH] Better Runtime error detection for bad paths --- lib/floe/validation_mixin.rb | 8 ++++++++ lib/floe/workflow/choice_rule/data.rb | 4 ++-- lib/floe/workflow/path.rb | 2 +- lib/floe/workflow/states/choice.rb | 6 ++++-- lib/floe/workflow/states/fail.rb | 6 +++--- lib/floe/workflow/states/input_output_mixin.rb | 10 +++++----- lib/floe/workflow/states/succeed.rb | 4 ++-- lib/floe/workflow/states/task.rb | 2 +- lib/floe/workflow/states/wait.rb | 6 +++--- spec/workflow/choice_rule_spec.rb | 6 +++--- spec/workflow/intrinsic_function_spec.rb | 2 +- spec/workflow/path_spec.rb | 4 ++-- spec/workflow/states/choice_spec.rb | 2 +- spec/workflow/states/pass_spec.rb | 6 +++--- 14 files changed, 39 insertions(+), 29 deletions(-) diff --git a/lib/floe/validation_mixin.rb b/lib/floe/validation_mixin.rb index d1ca24f3..82b6a628 100644 --- a/lib/floe/validation_mixin.rb +++ b/lib/floe/validation_mixin.rb @@ -22,6 +22,12 @@ def runtime_field_error!(field_name, field_value, comment, floe_error: "States.R raise Floe::ExecutionError.new(self.class.field_error_text(name, field_name, field_value, comment), floe_error) end + def wrap_runtime_error(field_name, field_value) + yield + rescue Floe::ExecutionError => 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 @@ -38,6 +44,8 @@ def parser_error!(name, comment) raise Floe::InvalidWorkflowError, "#{name} #{comment}" end + # these are here for State.build! + def missing_field_error!(name, field_name) parser_error!(name, "does not have required field \"#{field_name}\"") end diff --git a/lib/floe/workflow/choice_rule/data.rb b/lib/floe/workflow/choice_rule/data.rb index 9cbfd158..d282e095 100644 --- a/lib/floe/workflow/choice_rule/data.rb +++ b/lib/floe/workflow/choice_rule/data.rb @@ -68,7 +68,7 @@ def presence_check(context, input) # path found the variable_value, (so if they said true, return true) rhs - rescue Floe::PathError + rescue Floe::ExecutionError # variable_value (path) threw an error # it was not found (so if they said false, return true) !rhs @@ -117,7 +117,7 @@ def compare_value(context, input) end def fetch_path(field_name, field_path, context, input) - ret_value = field_path.value(context, input) + ret_value = wrap_runtime_error(field_name, field_path.to_s) { field_path.value(context, input) } runtime_field_error!(field_name, field_path.to_s, "must point to a #{type}") if type && !correct_type?(ret_value) ret_value end diff --git a/lib/floe/workflow/path.rb b/lib/floe/workflow/path.rb index c41a514b..6ae06b21 100644 --- a/lib/floe/workflow/path.rb +++ b/lib/floe/workflow/path.rb @@ -36,7 +36,7 @@ def value(context, input = {}) results = JsonPath.on(obj, path) case results.count when 0 - raise Floe::PathError, "Path [#{payload}] references an invalid value" + raise Floe::PathError, "references an invalid value" when 1 results.first else diff --git a/lib/floe/workflow/states/choice.rb b/lib/floe/workflow/states/choice.rb index c2c1b4fa..b373c04b 100644 --- a/lib/floe/workflow/states/choice.rb +++ b/lib/floe/workflow/states/choice.rb @@ -19,13 +19,15 @@ 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 runtime_field_error!("Default", nil, "not defined and no match found", :floe_error => "States.NoChoiceMatched") if next_state.nil? 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..dffd5508 100644 --- a/lib/floe/workflow/states/input_output_mixin.rb +++ b/lib/floe/workflow/states/input_output_mixin.rb @@ -5,8 +5,8 @@ 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 @@ -14,16 +14,16 @@ def process_output(context, results) return context.input.dup if results.nil? return if output_path.nil? - results = result_selector.value(context, results) if @result_selector + results = wrap_runtime_error("ResultSelector", @result_selector.to_s) { result_selector.value(context, results) } if @result_selector if result_path.payload.start_with?("$.Credentials") - credentials = result_path.set(context.credentials, results)["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) end - output_path.value(context, output) + wrap_runtime_error("OutputPath", output_path.to_s) { output_path.value(context, output) } 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 e2829d2a..491e7136 100644 --- a/spec/workflow/choice_rule_spec.rb +++ b/spec/workflow/choice_rule_spec.rb @@ -133,7 +133,7 @@ let(:input) { {} } it "raises an exception" do - expect { subject }.to raise_exception(Floe::PathError, "Path [$.foo] references an invalid value") + expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"Variable\" value \"$.foo\" references an invalid value") end end @@ -396,7 +396,7 @@ context "with path not found" do let(:input) { {"foo" => 2} } - it { expect { subject }.to raise_error(Floe::PathError, "Path [$.bar] references an invalid value") } + it { expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"NumericEqualsPath\" value \"$.bar\" references an invalid value") } end end @@ -581,7 +581,7 @@ context "that does not exist" do let(:input) { {} } - it { expect { subject }.to raise_exception(Floe::PathError, "Path [$.foo] references an invalid value") } + it { expect { subject }.to raise_exception(Floe::ExecutionError, "States.Choice1.Choices.0.Data field \"Variable\" value \"$.foo\" references an invalid value") } end context "that references a number" do diff --git a/spec/workflow/intrinsic_function_spec.rb b/spec/workflow/intrinsic_function_spec.rb index 513bf421..14a31fb4 100644 --- a/spec/workflow/intrinsic_function_spec.rb +++ b/spec/workflow/intrinsic_function_spec.rb @@ -656,7 +656,7 @@ it "handles invalid path references" do ctx = {"context" => {"baz" => "qux"}}, {"input" => {"foo" => "bar"}} - expect { described_class.value("States.Array($.xxx)", ctx) }.to raise_error(Floe::PathError, "Path [$.xxx] references an invalid value") + 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 a77d7e9a..f405fa4b 100644 --- a/spec/workflow/path_spec.rb +++ b/spec/workflow/path_spec.rb @@ -11,7 +11,7 @@ describe "#value" do context "referencing the global context" do it "with a missing value" do - expect { described_class.new("$$.foo").value({}, {"foo" => "bar"}) }.to raise_error(Floe::PathError, "Path [$$.foo] references an invalid value") + expect { described_class.new("$$.foo").value({}, {"foo" => "bar"}) }.to raise_error(Floe::PathError, "references an invalid value") end it "with a single value" 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 raise_error(Floe::PathError, "Path [$.foo] references an invalid value") + 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/states/choice_spec.rb b/spec/workflow/states/choice_spec.rb index 845306c2..328c37c2 100644 --- a/spec/workflow/states/choice_spec.rb +++ b/spec/workflow/states/choice_spec.rb @@ -50,7 +50,7 @@ expect(ctx.failed?).to eq(true) expect(ctx.output).to eq( { - "Cause" => "Path [$.foo] references an invalid value", + "Cause" => "States.Choice1.Choices.0.Data field \"Variable\" value \"$.foo\" references an invalid value", "Error" => "States.RuntimeError" } ) diff --git a/spec/workflow/states/pass_spec.rb b/spec/workflow/states/pass_spec.rb index 0376c877..db32c66f 100644 --- a/spec/workflow/states/pass_spec.rb +++ b/spec/workflow/states/pass_spec.rb @@ -168,7 +168,7 @@ expect(ctx.state_finished?).to eq(true) expect(ctx.output).to eq( { - "Cause" => "Path [$.missing] references an invalid value", + "Cause" => "States.PassState field \"InputPath\" value \"$.missing\" references an invalid value", "Error" => "States.RuntimeError" } ) @@ -191,7 +191,7 @@ expect(ctx.state_finished?).to eq(true) expect(ctx.output).to eq( { - "Cause" => "Path [$.missing.spot] references an invalid value", + "Cause" => "States.PassState field \"OutputPath\" value \"$.missing.spot\" references an invalid value", "Error" => "States.RuntimeError" } ) @@ -264,7 +264,7 @@ it "detects missing value" do workflow.run_nonblock - expect(ctx.output).to eq({"Cause" => "Path [$.color] references an invalid value", "Error" => "States.RuntimeError"}) + expect(ctx.output).to eq({"Cause" => "States.Pass field \"InputPath\" value \"$.color\" references an invalid value", "Error" => "States.RuntimeError"}) end end