Fix wrong logging message in AJ in case a job returns a falsey value:
- I made a change in 0d3aec4969
to output a log if a job was aborted
in a before callbacks. I didn't take in consideration that a job
could return a falsy value and thus it would wrongly log
that the job was aborted.
This fixes the problem by checking if the callback chain was halted
rather than the return value of the job.
This commit is contained in:
parent
2cd05dda2b
commit
ca61139fae
|
@ -18,12 +18,20 @@ module ActiveJob
|
|||
private
|
||||
def instrument(operation, payload = {}, &block)
|
||||
enhanced_block = ->(event_payload) do
|
||||
aborted = !block.call if block
|
||||
event_payload[:aborted] = true if aborted
|
||||
block.call if block
|
||||
if defined?(@_halted_callback_hook_called) && @_halted_callback_hook_called
|
||||
event_payload[:aborted] = true
|
||||
@_halted_callback_hook_called = nil
|
||||
end
|
||||
end
|
||||
|
||||
ActiveSupport::Notifications.instrument \
|
||||
"#{operation}.active_job", payload.merge(adapter: queue_adapter, job: self), &enhanced_block
|
||||
end
|
||||
|
||||
def halted_callback_hook(_)
|
||||
super
|
||||
@_halted_callback_hook_called = true
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -150,6 +150,35 @@ class LoggingTest < ActiveSupport::TestCase
|
|||
assert_match(/Error performing AbortBeforeEnqueueJob.* a before_perform callback halted/, @logger.messages)
|
||||
end
|
||||
|
||||
def test_perform_job_doesnt_log_error_when_job_returns_falsy_value
|
||||
job = Class.new(ActiveJob::Base) do
|
||||
def perform
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
subscribed { job.perform_now }
|
||||
assert_no_match(/Error performing AbortBeforeEnqueueJob.* a before_perform callback halted/, @logger.messages)
|
||||
end
|
||||
|
||||
def test_perform_job_doesnt_log_error_when_job_is_performed_multiple_times_and_fail_the_first_time
|
||||
job = Class.new(ActiveJob::Base) do
|
||||
before_perform do
|
||||
throw(:abort) if arguments[0].pop == :abort
|
||||
end
|
||||
|
||||
def perform(_)
|
||||
end
|
||||
end.new([:dont_abort, :abort])
|
||||
|
||||
subscribed do
|
||||
job.perform_now
|
||||
job.perform_now
|
||||
end
|
||||
|
||||
assert_equal(1, @logger.messages.scan(/a before_perform callback halted the job execution/).size)
|
||||
end
|
||||
|
||||
def test_perform_disabled_job_logging
|
||||
perform_enqueued_jobs do
|
||||
DisableLogJob.perform_later "Dummy"
|
||||
|
|
Loading…
Reference in New Issue