From 6072a0f510ba975e38c33dc584a805f46e683864 Mon Sep 17 00:00:00 2001 From: smudge Date: Thu, 3 Apr 2025 11:08:44 -0400 Subject: [PATCH 1/4] Define to_f, test explicit casting methods --- lib/delayed/priority.rb | 1 + spec/delayed/priority_spec.rb | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/lib/delayed/priority.rb b/lib/delayed/priority.rb index 129a176..fc67626 100644 --- a/lib/delayed/priority.rb +++ b/lib/delayed/priority.rb @@ -140,6 +140,7 @@ def method_missing(method_name, *args) attr_reader :value delegate :to_i, to: :value + delegate :to_f, to: :value delegate :to_s, to: :name def initialize(value) diff --git a/spec/delayed/priority_spec.rb b/spec/delayed/priority_spec.rb index c758896..7733283 100644 --- a/spec/delayed/priority_spec.rb +++ b/spec/delayed/priority_spec.rb @@ -207,6 +207,13 @@ expect(described_class.new(101)).to eq described_class.new(101) # rubocop:disable RSpec/IdenticalEqualityAssertion end + it 'supports explicit casting' do + expect(described_class.new(0).to_i).to eq 0 + expect(described_class.new(3).to_f).to eq 3.0 + expect(described_class.new(10).to_s).to eq 'user_visible' + expect(described_class.new(:eventual).to_d).to eq '20'.to_d + end + it 'suports coercion' do expect(described_class.new(0)).to eq 0 expect(described_class.new(8)).to be > 5 From a5df6e94b0a07a041389e273ed1746729fce3da1 Mon Sep 17 00:00:00 2001 From: smudge Date: Thu, 3 Apr 2025 11:16:44 -0400 Subject: [PATCH 2/4] Regression test for #53 --- spec/worker_spec.rb | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/worker_spec.rb b/spec/worker_spec.rb index 47ececa..6dcdc04 100644 --- a/spec/worker_spec.rb +++ b/spec/worker_spec.rb @@ -239,5 +239,37 @@ expect(performances.value).to eq(1) end + + it 'wraps perform and cleanup, even when perform raises' do + events = [] + + plugin = Class.new(Delayed::Plugin) do + callbacks do |lifecycle| + lifecycle.around(:thread) do |_, &blk| + events << :thread_start + blk.call + events << :thread_end + end + lifecycle.around(:perform) do |_, &blk| + events << :perform_start + blk.call + events << :perform_end + end + end + end + + Delayed.plugins << plugin + + job_class = Class.new do + def perform + raise 'boom' + end + end + + Delayed::Job.enqueue job_class.new + described_class.new.work_off + + expect(events).to eq %i(thread_start perform_start thread_end) + end end end From 1379d985fa09bb495668ef4cd704c01b02bd9b92 Mon Sep 17 00:00:00 2001 From: smudge Date: Thu, 3 Apr 2025 11:20:30 -0400 Subject: [PATCH 3/4] Priority#to_f, regression coverage, and 0.8.0 --- Gemfile.lock | 2 +- delayed.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 68f10cd..4986b9c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - delayed (0.7.2) + delayed (0.8.0) activerecord (>= 5.2) concurrent-ruby diff --git a/delayed.gemspec b/delayed.gemspec index baeefbc..e59ad80 100644 --- a/delayed.gemspec +++ b/delayed.gemspec @@ -18,7 +18,7 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.summary = 'a multi-threaded, SQL-driven ActiveJob backend used at Betterment to process millions of background jobs per day' - spec.version = '0.7.2' + spec.version = '0.8.0' spec.metadata = { 'changelog_uri' => 'https://github.com/betterment/delayed/blob/main/CHANGELOG.md', 'bug_tracker_uri' => 'https://github.com/betterment/delayed/issues', From f2a250c5e7c0f14e83b0a71f54b66ad224751f69 Mon Sep 17 00:00:00 2001 From: smudge Date: Thu, 3 Apr 2025 11:48:53 -0400 Subject: [PATCH 4/4] Assert job cleanup happens before end blocks --- lib/delayed/worker.rb | 20 ++++++++++---------- spec/worker_spec.rb | 20 +++++++++----------- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/lib/delayed/worker.rb b/lib/delayed/worker.rb index ae26da2..df9e306 100644 --- a/lib/delayed/worker.rb +++ b/lib/delayed/worker.rb @@ -142,17 +142,17 @@ def perform(job) job.destroy end job_say job, format('COMPLETED after %.4f seconds', run_time) + true # did work + rescue DeserializationError => e + job_say job, "FAILED permanently with #{e.class.name}: #{e.message}", 'error' + + job.error = e + failed(job) + false # work failed + rescue Exception => e # rubocop:disable Lint/RescueException + self.class.lifecycle.run_callbacks(:error, self, job) { handle_failed_job(job, e) } + false # work failed end - true # did work - rescue DeserializationError => e - job_say job, "FAILED permanently with #{e.class.name}: #{e.message}", 'error' - - job.error = e - failed(job) - false # work failed - rescue Exception => e # rubocop:disable Lint/RescueException - self.class.lifecycle.run_callbacks(:error, self, job) { handle_failed_job(job, e) } - false # work failed end # Reschedule the job in the future (when a job fails). diff --git a/spec/worker_spec.rb b/spec/worker_spec.rb index 6dcdc04..e917f0e 100644 --- a/spec/worker_spec.rb +++ b/spec/worker_spec.rb @@ -242,6 +242,7 @@ it 'wraps perform and cleanup, even when perform raises' do events = [] + last_error = nil plugin = Class.new(Delayed::Plugin) do callbacks do |lifecycle| @@ -250,26 +251,23 @@ blk.call events << :thread_end end - lifecycle.around(:perform) do |_, &blk| + lifecycle.around(:perform) do |_, job, &blk| events << :perform_start - blk.call - events << :perform_end + blk.call.tap do + last_error = job.last_error + events << :perform_end + end end end end Delayed.plugins << plugin - job_class = Class.new do - def perform - raise 'boom' - end - end - - Delayed::Job.enqueue job_class.new + Delayed::Job.enqueue ErrorJob.new described_class.new.work_off - expect(events).to eq %i(thread_start perform_start thread_end) + expect(events).to eq %i(thread_start perform_start perform_end thread_end) + expect(last_error).to match(/did not work/) # assert that cleanup happened before `:perform_end` end end end