github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-03 19:39:30 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

DEV: Add a try step to services

Browse Source 
This patch adds a new step to services named `try`.

It’s useful to rescue exceptions that some steps could raise. That way,
if an exception is caught, the service will stop its execution and can
be inspected like with any other steps.

Just wrap the steps that can raise with a `try` block:
```ruby
try do
  step :step_that_can_raise
  step :another_step_that_can_raise
end
```
By default, `try` will catch any exception inheriting from
`StandardError`, but we can specify what exceptions to catch:
```ruby
try(ArgumentError, RuntimeError) do
  step :will_raise
end
```

An outcome matcher has been added: `on_exceptions`. By default it will
be executed for any exception caught by the `try` step.
Here also, we can specify what exceptions to catch:
```ruby
on_exceptions(ArgumentError, RuntimeError) do |exception|
  …
end
```

Finally, an RSpec matcher has been added:
```ruby
  it { is_expected.to fail_with_exception }
  # or
  it { is_expected.to fail_with_exception(ArgumentError) }
```
This commit is contained in:
Loïc Guitaut
2024-11-08 17:24:40 +01:00
committed by Loïc Guitaut
parent 682e8df007
commit 719457e430
6 changed files with 295 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
lib/service/base.rb
View File

@ -121,6 +121,10 @@ module Service
const_set("Options", klass)
steps << OptionsStep.new(:default, class_name: klass)
end
def try(*exceptions, &block)
steps << TryStep.new(exceptions, &block)
end
end
# @!visibility private
@ -140,19 +144,19 @@ module Service
raise "In #{type} '#{name}': default values in step implementations are not allowed. Maybe they could be defined in a params or options block?"
end
args = context.slice(*method.parameters.select { _1[0] == :keyreq }.map(&:last))
context[result_key] = Context.build(object: object)
context[result_key] = Context.build({ object: }.compact)
instance.instance_exec(**args, &method)
end
def result_key
"result.#{type}.#{name}"
end
private
def type
self.class.name.split("::").last.downcase.sub(/^(\w+)step$/, "\\1")
end
def result_key
"result.#{type}.#{name}"
end
end
# @!visibility private
@ -244,6 +248,33 @@ module Service
end
end
# @!visibility private
class TryStep < Step
include StepsHelpers
attr_reader :steps, :exceptions
def initialize(exceptions, &block)
@name = "default"
@steps = []
@exceptions = exceptions.presence || [StandardError]
instance_exec(&block)
end
def call(instance, context)
context[result_key] = Context.build
steps.each do |step|
@current_step = step
step.call(instance, context)
end
rescue *exceptions => e
raise e if e.is_a?(Failure)
context[@current_step.result_key].fail(raised_exception?: true, exception: e)
context[result_key].fail(exception: e)
context.fail!
end
end
# @!visibility private
class OptionsStep < Step
def call(instance, context)

19
lib/service/runner.rb
View File

@ -22,10 +22,13 @@
# named `name` is not present
# * +on_model_errors(name)+: will execute the provided block if the model named
# `name` contains validation errors
# * +on_exceptions(*exceptions)+: will execute the provided block if any
# exceptions were caught by the `try` block. One or more exception classes
# can be provided to specifically handle those exceptions.
#
# All the specialized steps receive the failing step result object as an
# argument to their block. `on_model_errors` receives the actual model so it’s
# easier to inspect it.
# easier to inspect it, and `on_exceptions` receives the actual exception.
#
# @example In a controller
# def create
@ -92,6 +95,16 @@ class Service::Runner
key: [],
default_name: "model",
},
on_exceptions: {
condition: ->(*exceptions) do
next unless failure_for?("result.try.default")
next true if exceptions.empty?
exceptions.any? { result["result.try.default"].exception.is_a?(_1) }
end,
key: %w[result try],
name: "default",
property: :exception,
},
}.with_indifferent_access.freeze
# @!visibility private
@ -143,7 +156,9 @@ class Service::Runner
-> { instance_exec(*args, &action[:condition]) },
-> do
object.instance_exec(
result[[*action[:key], args.first || action[:default_name]].join(".")],
result[
[*action[:key], action[:name] || args.first || action[:default_name]].join(".")
].public_send(action[:property] || :itself),
**result.slice(*block.parameters.filter_map { _1.last if _1.first == :keyreq }),
&block
)

25
lib/service/steps_inspector.rb
View File

@ -10,7 +10,7 @@ class Service::StepsInspector
attr_reader :step, :result, :nesting_level
delegate :name, to: :step
delegate :failure?, :success?, :error, to: :step_result, allow_nil: true
delegate :failure?, :success?, :error, :raised_exception?, to: :step_result, allow_nil: true
def self.for(step, result, nesting_level: 0)
class_name =
@ -48,6 +48,7 @@ class Service::StepsInspector
end
def result_emoji
return "💥" if raised_exception?
return "❌" if failure?
return "✅" if success?
""
@ -96,18 +97,29 @@ class Service::StepsInspector
end
def inspect
"#{" " * nesting_level}[#{type}]"
"#{" " * nesting_level}[#{inspect_type}]"
end
def step_result
nil
end
end
#
# @!visibility private
class Options < Step
end
# @!visibility private
class Try < Transaction
def error
step_result.exception.inspect
end
def step_result
result["result.#{type}.#{name}"]
end
end
attr_reader :steps, :result
def initialize(result)
@ -123,7 +135,12 @@ class Service::StepsInspector
# [4/4] [step] 'change_status'
# @return [String] the steps of the result object with their state
def inspect
steps.map.with_index { |step, index| "[#{index + 1}/#{steps.size}] #{step.inspect}" }.join("\n")
steps
.map
.with_index do |step, index|
"[#{format("%#{steps.size.to_s.size}s", index + 1)}/#{steps.size}] #{step.inspect}"
end
.join("\n")
end
# @return [String, nil] the first available error, if any.

44
spec/lib/service/runner_spec.rb
View File

@ -147,6 +147,26 @@ RSpec.describe Service::Runner do
end
end
class FailureTryService
include Service::Base
try { step :raising_step }
def raising_step
raise "BOOM"
end
end
class SuccessTryService
include Service::Base
try { step :non_raising_step }
def non_raising_step
true
end
end
describe ".call" do
subject(:runner) { described_class.call(service, dependencies, &actions_block) }
@ -429,6 +449,30 @@ RSpec.describe Service::Runner do
end
end
context "when using the on_exceptions action" do
let(:actions) { <<-BLOCK }
proc do |result|
on_exceptions { |exception| exception.message == "BOOM" }
end
BLOCK
context "when the service fails" do
let(:service) { FailureTryService }
it "runs the provided block" do
expect(runner).to be true
end
end
context "when the service does not fail" do
let(:service) { SuccessTryService }
it "does not run the provided block" do
expect(runner).not_to eq true
end
end
end
context "when using several actions together" do
let(:service) { FailureService }
let(:actions) { <<-BLOCK }

177
spec/lib/service/steps_inspector_spec.rb
View File

@ -20,6 +20,7 @@ RSpec.describe Service::StepsInspector do
step :in_transaction_step_1
step :in_transaction_step_2
end
try { step :might_raise }
step :final_step
end
@ -30,9 +31,14 @@ RSpec.describe Service::StepsInspector do
before do
class DummyService
%i[fetch_model policy in_transaction_step_1 in_transaction_step_2 final_step].each do |name|
define_method(name) { true }
end
%i[
fetch_model
policy
in_transaction_step_1
in_transaction_step_2
might_raise
final_step
].each { |name| define_method(name) { true } }
end
end
@ -42,14 +48,16 @@ RSpec.describe Service::StepsInspector do
context "when service runs without error" do
it "outputs all the steps of the service" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ✅
[4/8] [params] 'default' ✅
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1' ✅
[7/8] [step] 'in_transaction_step_2' ✅
[8/8] [step] 'final_step' ✅
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ✅
[ 4/10] [params] 'default' ✅
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1' ✅
[ 7/10] [step] 'in_transaction_step_2' ✅
[ 8/10] [try]
[ 9/10] [step] 'might_raise' ✅
[10/10] [step] 'final_step' ✅
OUTPUT
end
end
@ -65,14 +73,16 @@ RSpec.describe Service::StepsInspector do
it "shows the failing step" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ❌
[3/8] [policy] 'policy'
[4/8] [params] 'default'
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1'
[7/8] [step] 'in_transaction_step_2'
[8/8] [step] 'final_step'
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ❌
[ 3/10] [policy] 'policy'
[ 4/10] [params] 'default'
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1'
[ 7/10] [step] 'in_transaction_step_2'
[ 8/10] [try]
[ 9/10] [step] 'might_raise'
[10/10] [step] 'final_step'
OUTPUT
end
end
@ -88,14 +98,16 @@ RSpec.describe Service::StepsInspector do
it "shows the failing step" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ❌
[4/8] [params] 'default'
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1'
[7/8] [step] 'in_transaction_step_2'
[8/8] [step] 'final_step'
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ❌
[ 4/10] [params] 'default'
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1'
[ 7/10] [step] 'in_transaction_step_2'
[ 8/10] [try]
[ 9/10] [step] 'might_raise'
[10/10] [step] 'final_step'
OUTPUT
end
end
@ -105,14 +117,16 @@ RSpec.describe Service::StepsInspector do
it "shows the failing step" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ✅
[4/8] [params] 'default' ❌
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1'
[7/8] [step] 'in_transaction_step_2'
[8/8] [step] 'final_step'
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ✅
[ 4/10] [params] 'default' ❌
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1'
[ 7/10] [step] 'in_transaction_step_2'
[ 8/10] [try]
[ 9/10] [step] 'might_raise'
[10/10] [step] 'final_step'
OUTPUT
end
end
@ -128,14 +142,41 @@ RSpec.describe Service::StepsInspector do
it "shows the failing step" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ✅
[4/8] [params] 'default' ✅
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1' ✅
[7/8] [step] 'in_transaction_step_2' ❌
[8/8] [step] 'final_step'
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ✅
[ 4/10] [params] 'default' ✅
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1' ✅
[ 7/10] [step] 'in_transaction_step_2' ❌
[ 8/10] [try]
[ 9/10] [step] 'might_raise'
[10/10] [step] 'final_step'
OUTPUT
end
end
context "when a step raises an exception inside the 'try' block" do
before do
class DummyService
def might_raise
raise "BOOM"
end
end
end
it "shows the failing step" do
expect(output).to eq <<~OUTPUT.chomp
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ✅
[ 4/10] [params] 'default' ✅
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1' ✅
[ 7/10] [step] 'in_transaction_step_2' ✅
[ 8/10] [try]
[ 9/10] [step] 'might_raise' 💥
[10/10] [step] 'final_step'
OUTPUT
end
end
@ -146,14 +187,16 @@ RSpec.describe Service::StepsInspector do
it "adapts its output accordingly" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ✅ ⚠️ <= expected to return false but got true instead
[4/8] [params] 'default' ✅
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1' ✅
[7/8] [step] 'in_transaction_step_2' ✅
[8/8] [step] 'final_step' ✅
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ✅ ⚠️ <= expected to return false but got true instead
[ 4/10] [params] 'default' ✅
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1' ✅
[ 7/10] [step] 'in_transaction_step_2' ✅
[ 8/10] [try]
[ 9/10] [step] 'might_raise' ✅
[10/10] [step] 'final_step' ✅
OUTPUT
end
end
@ -170,14 +213,16 @@ RSpec.describe Service::StepsInspector do
it "adapts its output accordingly" do
expect(output).to eq <<~OUTPUT.chomp
[1/8] [options] 'default' ✅
[2/8] [model] 'model' ✅
[3/8] [policy] 'policy' ❌ ⚠️ <= expected to return true but got false instead
[4/8] [params] 'default'
[5/8] [transaction]
[6/8] [step] 'in_transaction_step_1'
[7/8] [step] 'in_transaction_step_2'
[8/8] [step] 'final_step'
[ 1/10] [options] 'default' ✅
[ 2/10] [model] 'model' ✅
[ 3/10] [policy] 'policy' ❌ ⚠️ <= expected to return true but got false instead
[ 4/10] [params] 'default'
[ 5/10] [transaction]
[ 6/10] [step] 'in_transaction_step_1'
[ 7/10] [step] 'in_transaction_step_2'
[ 8/10] [try]
[ 9/10] [step] 'might_raise'
[10/10] [step] 'final_step'
OUTPUT
end
end
@ -266,5 +311,19 @@ RSpec.describe Service::StepsInspector do
expect(error).to eq("my error")
end
end
context "when an exception occurred inside the 'try' block" do
before do
class DummyService
def might_raise
raise "BOOM"
end
end
end
it "returns an error related to the exception" do
expect(error).to match(/RuntimeError: BOOM/)
end
end
end
end

63
spec/support/service_matchers.rb
View File

@ -47,9 +47,9 @@ module ServiceMatchers
set_unexpected_result
message =
if !step_exists?
"Expected #{type} '#{name}' (key: '#{step}') was not found in the result object."
step_not_existing_message
elsif !step_failed?
"Expected #{type} '#{name}' (key: '#{step}') to fail but it succeeded."
step_failed_message
else
"expected the service to fail but it succeeded."
end
@ -58,8 +58,7 @@ module ServiceMatchers
def failure_message_when_negated
set_unexpected_result
message = "Expected #{type} '#{name}' (key: '#{step}') to succeed but it failed."
error_message_with_inspection(message)
error_message_with_inspection(negated_message)
end
def description
@ -97,6 +96,18 @@ module ServiceMatchers
return unless result[step]
result[step]["spec.unexpected_result"] = true
end
def step_not_existing_message
"Expected #{type} '#{name}' (key: '#{step}') was not found in the result object."
end
def step_failed_message
"Expected #{type} '#{name}' (key: '#{step}') to fail but it succeeded."
end
def negated_message
"Expected #{type} '#{name}' (key: '#{step}') to succeed but it failed."
end
end
class FailContract < FailStep
@ -133,6 +144,46 @@ module ServiceMatchers
end
end
class FailWithException < FailStep
attr_reader :exception
def initialize(exception)
@exception = exception
@name = "default"
end
def type
"try"
end
def description
"fail with an exception (#{exception})"
end
def step_failed?
super && result[step].exception.is_a?(exception)
end
def step_not_existing_message
"Expected try block (key: '#{step}') was not found in the result object."
end
def step_failed_message
message =
"Expected try block (key: '#{step}') to fail with an exception of type '#{exception}'"
message +=
if result[step].exception.blank?
" but it succeeded."
else
" but it failed with an exception of type '#{result[step].exception.class}'"
end
end
def negated_message
"Expected try block (key: '#{step}') to succeed but it failed."
end
end
def fail_a_policy(name)
FailPolicy.new(name)
end
@ -149,6 +200,10 @@ module ServiceMatchers
FailWithInvalidModel.new(name)
end
def fail_with_exception(exception = StandardError)
FailWithException.new(exception)
end
def fail_a_step(name = "model")
FailStep.new(name)
end