Skip to content

Commit

Permalink
Avoid setting status to jobs that are not Sidekiq::Status::Worker upo…
Browse files Browse the repository at this point in the history
…n exceptions (#32)
  • Loading branch information
monsha authored Apr 2, 2023
1 parent bf42342 commit 0e3acf4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 18 deletions.
38 changes: 20 additions & 18 deletions lib/sidekiq-status/server_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,28 @@ def call(worker, msg, queue)
return
end

# Determine job expiration
expiry = job_class.new.expiration || @expiration rescue @expiration

store_status worker.jid, :working, expiry
yield
store_status worker.jid, :complete, expiry
rescue Worker::Stopped
store_status worker.jid, :stopped, expiry
rescue SystemExit, Interrupt
store_status worker.jid, :interrupted, expiry
raise
rescue Exception
status = :failed
if msg['retry']
if retry_attempt_number(msg) < retry_attempts_from(msg['retry'], DEFAULT_MAX_RETRY_ATTEMPTS)
status = :retrying
begin
# Determine job expiration
expiry = job_class.new.expiration || @expiration rescue @expiration

store_status worker.jid, :working, expiry
yield
store_status worker.jid, :complete, expiry
rescue Worker::Stopped
store_status worker.jid, :stopped, expiry
rescue SystemExit, Interrupt
store_status worker.jid, :interrupted, expiry
raise
rescue Exception
status = :failed
if msg['retry']
if retry_attempt_number(msg) < retry_attempts_from(msg['retry'], DEFAULT_MAX_RETRY_ATTEMPTS)
status = :retrying
end
end
store_status(worker.jid, status, expiry) if job_class && job_class.ancestors.include?(Sidekiq::Status::Worker)
raise
end
store_status(worker.jid, status, expiry) if job_class && job_class.ancestors.include?(Sidekiq::Status::Worker)
raise
end

private
Expand Down
16 changes: 16 additions & 0 deletions spec/lib/sidekiq-status/server_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@
end
expect(redis.hget("sidekiq:status:#{job_id}", :status)).to be_nil
end

it "should not set any status on system exit signal" do
allow(SecureRandom).to receive(:hex).once.and_return(job_id)
start_server do
expect(ExitedNoStatusJob.perform_async).to eq(job_id)
end
expect(redis.hget("sidekiq:status:#{job_id}", :status)).to be_nil
end

it "should not set any status on interrupt signal" do
allow(SecureRandom).to receive(:hex).once.and_return(job_id)
start_server do
expect(InterruptedNoStatusJob.perform_async).to eq(job_id)
end
expect(redis.hget("sidekiq:status:#{job_id}", :status)).to be_nil
end
end

context "sets interrupted status" do
Expand Down
12 changes: 12 additions & 0 deletions spec/support/test_jobs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,24 @@ def perform
end
end

class ExitedNoStatusJob < StubNoStatusJob
def perform
raise SystemExit
end
end

class InterruptedJob < StubJob
def perform
raise Interrupt
end
end

class InterruptedNoStatusJob < StubNoStatusJob
def perform
raise Interrupt
end
end

class RetriedJob < StubJob

sidekiq_options retry: true
Expand Down

0 comments on commit 0e3acf4

Please sign in to comment.