Skip to content

Commit

Permalink
Better Runtime error detection for bad paths
Browse files Browse the repository at this point in the history
  • Loading branch information
kbrock committed Jul 15, 2024
1 parent f8b256e commit cf60580
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 29 deletions.
8 changes: 8 additions & 0 deletions lib/floe/validation_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/floe/workflow/choice_rule/data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/floe/workflow/path.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions lib/floe/workflow/states/choice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions lib/floe/workflow/states/fail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions lib/floe/workflow/states/input_output_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +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.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
Expand Down
4 changes: 2 additions & 2 deletions lib/floe/workflow/states/succeed.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/floe/workflow/states/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/floe/workflow/states/wait.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions spec/workflow/choice_rule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/workflow/intrinsic_function_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions spec/workflow/path_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/workflow/states/choice_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
)
Expand Down
6 changes: 3 additions & 3 deletions spec/workflow/states/pass_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
)
Expand All @@ -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"
}
)
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit cf60580

Please sign in to comment.