ruby-shoryuken / shoryuken

A super efficient Amazon SQS thread based message processor for Ruby. This project is in MAINTENANCE MODE. Reach me out on Slack link on the description if you want to become a new maintainer.
Other
2.06k stars 280 forks source link

Add Support for Non-retryable Exception to Shoyuken #734

Closed pyueli closed 1 year ago

pyueli commented 1 year ago

I am working on Shoyuken recently and found the default exponential_backoff_retry middleware doesn't support non-retryable Exception. It will retry the failed message multiple times with intervals specified by retry_intervals. One idea is to add a new non_retryable_exception middleware, which allows user specify the list of non-retryable exceptions in the worker. This new middleware can be added after the exponential_backoff_retry middleware.

Below is the pseudo code.

    def call(worker, _queue, sqs_msg, _body)
      yield
    rescue => ex
       if ex in nonretryable_exceptions
          sqs_msg.delete
          logger.warn ...
          return
       end 
       raise

I am happy to work on a PR to solve this issue. Below are two sequence diagrams showing how the new middleware handles the worker exceptions based on the exception type. Any thoughts? Thx!

Screenshot 2023-05-09 at 11 28 05 AM Screenshot 2023-05-09 at 11 28 18 AM
phstc commented 1 year ago

Hi @pyueli

Do you have many workers that you want to ignore retrying? If it's a one-off case, I would recommend the approach you suggested.

class Worker
  include Shoryuken::Worker

  shoryuken_options retry_intervals: [300, 1200, 3600] # 5.minutes, 20.minutes and 1.hour

  def perform(sqs_msg, name)
    # do something
  rescue SomeError => ex
    # ignore some error
  end
end

If I understood your diagram correctly, non_retryable_exception would be called right after retry_intervals. This additional request would be voided.

pyueli commented 1 year ago

Hi @phstc Thanks for your reply. Yes. There are multiple cases for this requirment. So I am trying to build this feature as a new middleware in Shoryuken instead of duplicating the exception handling logic in the worker.

If I understood your diagram correctly, non_retryable_exception would be called right after retry_intervals. This additional request would be voided.

This new middleware handles specified non_retryable exceptions. So those exceptions won't be passed to exponential_backoff_retry middleware

github-actions[bot] commented 1 year ago

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

github-actions[bot] commented 1 year ago

This issue was closed because it hasn't seen activity for a while.