Skip to content

Fix handling when the error changes after a retry#970

Merged
ryz310 merged 2 commits intoryz310:masterfrom
ashimomura:bugfix/fix-rescue-with-handler
Jan 9, 2024
Merged

Fix handling when the error changes after a retry#970
ryz310 merged 2 commits intoryz310:masterfrom
ashimomura:bugfix/fix-rescue-with-handler

Conversation

@ashimomura
Copy link
Contributor

@ashimomura ashimomura commented Jan 8, 2024

Motivation / Background

This gem utilizes retry_on with ActiveSupport::Rescuable for implementation, but the behavior of ActiveSupport::Rescuable.rescue_with_handler has changed since ActiveSupport v5.2.

Since ActiveSupport 5.2, if no handler exists for the exception passed as an argument, it searches for a handler attached to the #cause of that exception.

https://github.com/rails/rails/blob/v5.2.0/activesupport/lib/active_support/rescuable.rb#L88-L101

Therefore, even when the exception changes after being retried due to a retryable exception,
it continues to be processed as a retryable exception because it searches for a handler attached to the #cause of the exception.

e.g.

class Example
  class RetriableError < StandardError; end
  class OtherError < StandardError; end

  include MyApiClient::Exceptions

  retry_on RetriableError, wait: 0.seconds, attempts: 2

  private

  def execute
    puts 'execute'
    unless @first_executed
      @first_executed = true
      raise RetriableError
    else
      raise OtherError
    end
  end
end

instance = Example.new
instance.call(:execute) # => 1. raised RetriableError, and retry
                        # => 2. raised OtherError, handler not found. but searches for a handler of `Exception#cause` that is RetryableError
                        # => 3. continues retry 

Detail

Override ActiveSupport::Rescuable.rescue_with_handler and pass Exception#cause to visited_exceptions to treat it as a "handled" exception.

Copy link
Owner

@ryz310 ryz310 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

@ryz310 ryz310 merged commit f0b1427 into ryz310:master Jan 9, 2024
@ashimomura ashimomura deleted the bugfix/fix-rescue-with-handler branch January 10, 2024 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants