Skip to content

Commit

Permalink
Ignore ActiveJob::DeserializationError when discarding jobs (#771)
Browse files Browse the repository at this point in the history
Previously, discarding a job would attempt to deserialize the job
arguments and crash. We load the job to attempt to run the `#instrument`
method, but unfortunately this means that the job could not be
discarded.

This commit adds a flag to `Execution#active_job`,
`ignore_deserialization_errors`, which says "I want to load this job but
I don't mind if the arguments cannot be deserialized".

There are only two places currently where this method is called, and the
other one (`retry_job`) will still throw errors when attempting to retry
a job in this state. But now, when discarding, the errors are completely
ignored and the job can be discarded.
  • Loading branch information
nickcampbell18 authored Dec 12, 2022
1 parent 39d7e8b commit 78dfb47
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
9 changes: 8 additions & 1 deletion app/models/good_job/execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,16 @@ def executable?
self.class.unscoped.unfinished.owns_advisory_locked.exists?(id: id)
end

def active_job
# Build an ActiveJob instance and deserialize the arguments, using `#active_job_data`.
#
# @param ignore_deserialization_errors [Boolean]
# Whether to ignore ActiveJob::DeserializationError when deserializing the arguments.
# This is most useful if you aren't planning to use the arguments directly.
def active_job(ignore_deserialization_errors: false)
ActiveJob::Base.deserialize(active_job_data).tap do |aj|
aj.send(:deserialize_arguments_if_needed)
rescue ActiveJob::DeserializationError
raise unless ignore_deserialization_errors
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def retry_job
def discard_job(message)
with_advisory_lock do
execution = head_execution(reload: true)
active_job = execution.active_job
active_job = execution.active_job(ignore_deserialization_errors: true)

raise ActionForStateMismatchError if execution.finished_at.present?

Expand Down
22 changes: 22 additions & 0 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,28 @@ def perform(feline = nil, canine: nil)
expect { job.discard_job("Discard in test") }.to raise_error GoodJob::Job::ActionForStateMismatchError
end
end

context 'when job arguments cannot be deserialized' do
let(:deserialization_exception) do
# `ActiveJob::DeserializationError` looks at `$!` (last exception), so to create
# an instance of this exception we need to trigger a canary exception first.
original_exception = StandardError.new("Unsupported argument")
begin
raise original_exception
rescue StandardError
ActiveJob::DeserializationError.new
end
end

it 'ignores the error and discards the job' do
allow_any_instance_of(ActiveJob::Base).to receive(:deserialize_arguments_if_needed).and_raise(deserialization_exception)
expect_any_instance_of(ActiveJob::Base).to receive(:deserialize_arguments_if_needed)

expect do
job.discard_job("Discarded in test")
end.to change { job.reload.status }.from(:scheduled).to(:discarded)
end
end
end

describe '#reschedule_job' do
Expand Down

0 comments on commit 78dfb47

Please sign in to comment.