From 8b4ef70a8a5cfa3d4d5a25ac253b945db711906f Mon Sep 17 00:00:00 2001 From: Dmitriy Ivliev <3938172+moofkit@users.noreply.github.com> Date: Thu, 30 May 2024 12:39:40 +0300 Subject: [PATCH] fix bug with dsl options inheritance --- lib/sidekiq/rescue/dsl.rb | 11 +++++--- lib/sidekiq/rescue/rspec/matchers.rb | 2 +- lib/sidekiq/rescue/server_middleware.rb | 4 +-- spec/sidekiq/rescue/dsl_spec.rb | 36 +++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/lib/sidekiq/rescue/dsl.rb b/lib/sidekiq/rescue/dsl.rb index e36b32f..23d3d76 100644 --- a/lib/sidekiq/rescue/dsl.rb +++ b/lib/sidekiq/rescue/dsl.rb @@ -31,8 +31,13 @@ def sidekiq_rescue(*errors, delay: Sidekiq::Rescue.config.delay, limit: Sidekiq: assign_sidekiq_rescue_options(unpacked_errors, delay, limit) end - def sidekiq_rescue_options_for(error) - sidekiq_rescue_options&.find { |k, _v| k.include?(error) }&.last + # Find the error group and options for the given exception. + # @param exception [StandardError] The exception to find the error group for. + # @return [Array, Hash] The error group and options. + def sidekiq_rescue_error_group_with_options_by(exception) + sidekiq_rescue_options.reverse_each.find do |error_group, _options| + Array(error_group).any? { |error_klass| exception.is_a?(error_klass) } + end end private @@ -65,7 +70,7 @@ def validate_limit_argument(limit) def assign_sidekiq_rescue_options(errors, delay, limit) self.sidekiq_rescue_options ||= {} - self.sidekiq_rescue_options.merge!(errors => { delay: delay, limit: limit }) + self.sidekiq_rescue_options = self.sidekiq_rescue_options.merge(errors => { delay: delay, limit: limit }) end end end diff --git a/lib/sidekiq/rescue/rspec/matchers.rb b/lib/sidekiq/rescue/rspec/matchers.rb index 226b09f..38d6efd 100644 --- a/lib/sidekiq/rescue/rspec/matchers.rb +++ b/lib/sidekiq/rescue/rspec/matchers.rb @@ -35,7 +35,7 @@ module Matchers return false unless matched - options = actual.sidekiq_rescue_options_for(expected) + _error_group, options = actual.sidekiq_rescue_error_group_with_options_by(expected.new) (@delay.nil? || options.fetch(:delay) == @delay) && (@limit.nil? || options.fetch(:limit) == @limit) diff --git a/lib/sidekiq/rescue/server_middleware.rb b/lib/sidekiq/rescue/server_middleware.rb index f6f69d1..e32a46d 100644 --- a/lib/sidekiq/rescue/server_middleware.rb +++ b/lib/sidekiq/rescue/server_middleware.rb @@ -23,9 +23,7 @@ def call(job_instance, job_payload, _queue, &block) def sidekiq_rescue(job_payload, job_class) yield rescue StandardError => e - error_group, options = job_class.sidekiq_rescue_options.reverse_each.find do |error_group, _options| - Array(error_group).any? { |error| e.is_a?(error) } - end + error_group, options = job_class.sidekiq_rescue_error_group_with_options_by(e) raise e unless error_group rescue_error(e, error_group, options, job_payload) diff --git a/spec/sidekiq/rescue/dsl_spec.rb b/spec/sidekiq/rescue/dsl_spec.rb index 68b02bf..313688d 100644 --- a/spec/sidekiq/rescue/dsl_spec.rb +++ b/spec/sidekiq/rescue/dsl_spec.rb @@ -107,5 +107,41 @@ def define_dsl(...) "limit must be integer" ) end + + it "inherits the parent's options in right order" do + parent = Class.new do + include Sidekiq::Job + include Sidekiq::Rescue::Dsl + + sidekiq_rescue ParentError, delay: 10 + end + + child = Class.new(parent) do + sidekiq_rescue ChildError, delay: 20 + end + + expect(parent.sidekiq_rescue_options).to eq({ [ParentError] => { delay: 10, limit: 10 } }) + expect(child.sidekiq_rescue_options.to_a).to eq([[[ParentError], { delay: 10, limit: 10 }], + [[ChildError], { delay: 20, limit: 10 }]]) + end + end + + describe "#sidekiq_rescue_error_group_with_options_by" do + it "returns the options for the error" do + parent = Class.new do + include Sidekiq::Job + include Sidekiq::Rescue::Dsl + + sidekiq_rescue ParentError, delay: 10 + end + + child = Class.new(parent) do + sidekiq_rescue ChildError, delay: 20 + end + + expect(parent.sidekiq_rescue_error_group_with_options_by(ParentError.new).last).to eq(delay: 10, limit: 10) + expect(child.sidekiq_rescue_error_group_with_options_by(ParentError.new).last).to eq(delay: 10, limit: 10) + expect(child.sidekiq_rescue_error_group_with_options_by(ChildError.new).last).to eq(delay: 20, limit: 10) + end end end