From 702902927da43f21f82108b9fb07a7aa858f0385 Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Mon, 7 Nov 2016 02:34:11 +0000 Subject: [PATCH] Don't shadow job method in Processor (#3225) The Processor class has a `#job` method which was being shadowed in `#process` by a local variable. The `#process` method is complex, and I recently moved some code around in a way that accidentally resulted in the instance method being used instead of the local variable. By using a different name for the variable, making that same mistake again will result in a `NameError` instead of doing the wrong thing. --- lib/sidekiq/processor.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/sidekiq/processor.rb b/lib/sidekiq/processor.rb index 15e9f24b..e1326b59 100644 --- a/lib/sidekiq/processor.rb +++ b/lib/sidekiq/processor.rb @@ -121,19 +121,19 @@ module Sidekiq ack = false begin - job = Sidekiq.load_json(jobstr) + job_hash = Sidekiq.load_json(jobstr) @reloader.call do - klass = job['class'.freeze].constantize + klass = job_hash['class'.freeze].constantize worker = klass.new - worker.jid = job['jid'.freeze] + worker.jid = job_hash['jid'.freeze] - stats(worker, job, queue) do - Sidekiq.server_middleware.invoke(worker, job, queue) do + stats(worker, job_hash, queue) do + Sidekiq.server_middleware.invoke(worker, job_hash, queue) do # Only ack if we either attempted to start this job or # successfully completed it. This prevents us from # losing jobs if a middleware raises an exception before yielding ack = true - execute_job(worker, cloned(job['args'.freeze])) + execute_job(worker, cloned(job_hash['args'.freeze])) end end ack = true @@ -144,7 +144,7 @@ module Sidekiq # we didn't properly finish it. ack = false rescue Exception => ex - handle_exception(ex, { :context => "Job raised exception", :job => job, :jobstr => jobstr }) + handle_exception(ex, { :context => "Job raised exception", :job => job_hash, :jobstr => jobstr }) raise ensure work.acknowledge if ack @@ -163,9 +163,9 @@ module Sidekiq PROCESSED = Concurrent::AtomicFixnum.new FAILURE = Concurrent::AtomicFixnum.new - def stats(worker, job, queue) + def stats(worker, job_hash, queue) tid = thread_identity - WORKER_STATE[tid] = {:queue => queue, :payload => cloned(job), :run_at => Time.now.to_i } + WORKER_STATE[tid] = {:queue => queue, :payload => cloned(job_hash), :run_at => Time.now.to_i } begin yield