Better Runtime error detection for bad paths

kbrock · kbrock · commit 70b31d32e3bf · 2025-09-26T17:04:25.000-04:00
diff --git 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
diff --git a/lib/floe/workflow/choice_rule/data.rb b/lib/floe/workflow/choice_rule/data.rb
@@ -51,7 +51,7 @@ def true?(context, input)
           rhs = compare_value(context, input)
           lhs = variable_value(context, input)
           send(OPERATIONS[operator], lhs, rhs)
-        rescue Floe::PathError
+        rescue Floe::ExecutionError
           # For IsPresent, we can expect the lhs to not be present in some cases,
           #                This throws a PathError. We handle that special case here.
           # Example:
@@ -188,7 +188,7 @@ def parse_field(field_name, data_type)
 
         # fetch a path at runtime
         def fetch_path(field_name, field_path, context, input)
-          value = field_path.value(context, input)
+          value =  wrap_runtime_error(field_name, field_path.to_s) { field_path.value(context, input) }
           # if this is an operation (GreaterThanPath), ensure the value is the correct type
           return value if type.nil? || correct_type?(value, type)
 
diff --git a/lib/floe/workflow/intrinsic_function/transformer.rb b/lib/floe/workflow/intrinsic_function/transformer.rb
@@ -75,7 +75,7 @@ def check_types(args, function, signature)
 
         rule(:string   => simple(:v)) { v.to_s[1..-2] }
         rule(:number   => simple(:v)) { v.match(/[eE.]/) ? Float(v) : Integer(v) }
-        rule(:jsonpath => simple(:v)) { Floe::Workflow::Path.value(v.to_s, context, input) }
+        rule(:jsonpath => simple(:v)) { Floe::Workflow::Path.value(v.to_s, context, input) } # improve error here
 
         STATES_FORMAT_PLACEHOLDER = /(?<!\\)\{\}/
 
diff --git 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
@@ -18,8 +18,8 @@ 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) }
           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?
diff --git a/lib/floe/workflow/states/fail.rb b/lib/floe/workflow/states/fail.rb
@@ -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
@@ -5,24 +5,25 @@ 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 = 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.match?(/^\$\$\.Credentials\b/)
-            context.credentials.merge!(result_path.set(context.credentials, results))
+            credentials = wrap_runtime_error("ResultPath", result_path.to_s) { result_path.set(context.credentials, results) }
+            context.credentials.merge!(credentials) if 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/map.rb b/lib/floe/workflow/states/map.rb
@@ -43,7 +43,7 @@ def initialize(workflow, name, payload)
 
         def process_input(context)
           input = super
-          input = items_path.value(context, input)
+          input = wrap_runtime_error("ItemPath", items_path.to_s) { items_path.value(context, input) }
           input = item_batcher.value(context, input, context.state["Input"]) if item_batcher
           input
         end
diff --git a/lib/floe/workflow/states/retry_catch_mixin.rb b/lib/floe/workflow/states/retry_catch_mixin.rb
@@ -38,7 +38,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) }
           context.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/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/wait.rb b/lib/floe/workflow/states/wait.rb
@@ -28,18 +28,18 @@ 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,
-            :seconds => seconds_path ? seconds_path.value(context, input).to_i : seconds,
-            :time    => timestamp_path ? timestamp_path.value(context, input) : timestamp
+            :seconds => seconds_path ? wrap_runtime_error("SecondsPath", seconds_path.to_s) { seconds_path.value(context, input).to_i } : seconds,
+            :time    => timestamp_path ? wrap_runtime_error("TimestampPath", timestamp_path.to_s) { timestamp_path.value(context, input) } : timestamp
           )
         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
@@ -138,7 +138,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
 
@@ -408,7 +408,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
 
@@ -593,7 +593,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
@@ -195,7 +195,7 @@
       end
 
       it "fails with invalid argument values" do
-        expect { described_class.value("States.StringToJson($.input)", {}, {"input" => "foo"}) }.to raise_error(ArgumentError, /invalid value for argument 1 to States.StringToJson \(invalid json/)
+        expect { described_class.value("States.StringToJson($.input)", {}, {"input" => "foo"}) }.to raise_error(ArgumentError, /invalid value for argument 1 to States\.StringToJson \(invalid json: unexpected token .*\)/)
       end
     end
 
@@ -990,7 +990,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
@@ -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
@@ -36,7 +36,7 @@
       let(:input)   { {} }
 
       it "with a missing value" do
-        expect { described_class.new("$$.Credentials.username").value(context, input) }.to raise_error(Floe::PathError, "Path [$$.Credentials.username] references an invalid value")
+        expect { described_class.new("$$.Credentials.username").value(context, input) }.to raise_error(Floe::PathError, "references an invalid value")
       end
 
       it "returns the password" do
@@ -46,7 +46,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
@@ -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.Runtime"
           }
         )
diff --git 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.Runtime"
           }
         )
@@ -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.Runtime"
           }
         )
@@ -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.Runtime"})
+          expect(ctx.output).to eq({"Cause" => "States.Pass field \"InputPath\" value \"$.color\" references an invalid value", "Error" => "States.Runtime"})
         end
       end
 
diff --git a/spec/workflow_spec.rb b/spec/workflow_spec.rb
@@ -40,10 +40,12 @@
       expect { described_class.new(payload) }.to raise_error(Floe::InvalidWorkflowError, "State Machine field \"StartAt\" value \"Foo\" is not found in \"States\"")
     end
 
+    # this error gets truncated when embedded in the parent string, so is basically useless
     it "raises an exception for invalid context" do
       payload = {"StartAt" => "FirstState", "States" => {"FirstState" => {"Type" => "Succeed"}}}
 
-      expect { described_class.new(payload, "invalid context") }.to raise_error(Floe::InvalidExecutionInput, /Invalid State Machine Execution Input: unexpected character: /)
+      expect { described_class.new(payload, "invalid context") }.to raise_error(Floe::InvalidExecutionInput,
+        /unexpected.*'invalid.*: was expecting \(JSON String, Number, Array, Object or token 'null', 'true' or 'false'\)/)
     end
   end
 
