breamware / sidekiq-batch

Sidekiq Batch Jobs Implementation
MIT License
357 stars 95 forks source link

Success Callback always triggered for single failing job #27

Closed Jay-Plumb closed 4 years ago

Jay-Plumb commented 5 years ago

Hey everyone. There is an issue when I create a batch and supply a single job to it. Let me supply some code for anyone to replicate:

# app/workers/create_cluster_worker.rb
class CreateClusterWorker

  def perform
    batch = Sidekiq::Batch.new
    batch.description = 'test'
    batch.on(:success, CreateClusterWorker::Created, { "cluster_id" => 999 })
    batch.on(:complete, CreateClusterWorker::Created, { "cluster_id" => 999 })

    batch.jobs do
      1.times { |i| CreateServerWorker.perform_async(i) }
    end
  end

  class Created
    def on_success(status, options)
      puts status, options
    end

    def on_complete(status, options)
      puts status, options
    end
  end
end
# app/workers/create_server_worker.rb
class CreateServerWorker
  include Sidekiq::Worker
  def perform(id)
    i = rand(1..10)
    sleep i
    raise StandardError unless 'a'.eql? 'b' # Deliberatly create a failure
  end
end

Executing CreateClusterWorker.new.perform from the rails console triggers the on_complete callback (which correctly passes status.failures to equal 1) but also triggers the on_success callback (which incorrecly passes status.failures to equal 0).

Environment

sidekiq-batch (0.1.4)
sidekiq (5.0.4)
redis (4.1.0)
ruby (2.4.4)
rails (5.2.2)

Disclaimer

https://github.com/breamware/sidekiq-batch/pull/26 might fix the issue but given that the branch has merge conflicts I am unable to confirm. I wanted to create this issue just incase it is a slightly different edge case to the currently open issues.

nglx commented 5 years ago

@Jay-Plumb could you check if the code from the branch https://github.com/breamware/sidekiq-batch/tree/jbrady42_pr_fixes is working fine? I'll try to fix failing specs and merge it to master hopefully sometime this weekend.

Jay-Plumb commented 5 years ago

@managr I was on a work retreat last week so apologies for the late response. I will have a check on https://github.com/breamware/sidekiq-batch/tree/jbrady42_pr_fixes during my work day tomorrow :)

Jay-Plumb commented 5 years ago

@managr Using the branch https://github.com/breamware/sidekiq-batch/tree/jbrady42_pr_fixes does not fix the issue. When CreateServerWorker runs without a retry (by not raising StandardError), the on_complete callback fires but the on_success does not.

For some reason, the on_success callback is never fired...

The same is true when multiple jobs are queued e.g:

batch.jobs do
  2.times { |i| CreateServerWorker.perform_async(i) }
end
github-actions[bot] commented 4 years ago

Stale issue message