From c6f2459cc46c705e0ac26188c4b49565d3c1b908 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 8 Jul 2021 12:49:58 +1000 Subject: [PATCH] FIX: Do not prevent other topic timers running on error (#13665) There was an issue with the TopicTimerEnqueuer where any timer that failed to enqueue_typed_job with an error would prevent all other pending timers after the one that errored from running. To mitigate this we just capture the error and log it (so we can still fix it if needed for bug crushing) and proceed with the rest of the timer enqueues. The commit https://github.com/discourse/discourse/pull/13544 highlighted this issue originally in hosted sites. --- app/jobs/scheduled/topic_timer_enqueuer.rb | 10 ++++++---- spec/jobs/topic_timer_enqueuer_spec.rb | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/jobs/scheduled/topic_timer_enqueuer.rb b/app/jobs/scheduled/topic_timer_enqueuer.rb index 7fa2889b1f3..67204c4200c 100644 --- a/app/jobs/scheduled/topic_timer_enqueuer.rb +++ b/app/jobs/scheduled/topic_timer_enqueuer.rb @@ -12,13 +12,15 @@ module Jobs every 1.minute def execute(_args = nil) - timers = TopicTimer.pending_timers - - timers.find_each do |timer| + TopicTimer.pending_timers.find_each do |timer| # the typed job may not enqueue if it has already # been scheduled with enqueue_at - timer.enqueue_typed_job + begin + timer.enqueue_typed_job + rescue => err + Discourse.warn_exception(err, message: "Error when attempting to enqueue topic timer job for timer #{timer.id}") + end end end end diff --git a/spec/jobs/topic_timer_enqueuer_spec.rb b/spec/jobs/topic_timer_enqueuer_spec.rb index b9f21ed12c8..702353be177 100644 --- a/spec/jobs/topic_timer_enqueuer_spec.rb +++ b/spec/jobs/topic_timer_enqueuer_spec.rb @@ -47,4 +47,9 @@ RSpec.describe Jobs::TopicTimerEnqueuer do Jobs.enqueue_at(1.hours.from_now, :close_topic, topic_timer_id: timer1.id) subject.execute end + + it "does not fail to enqueue other timers just because one timer errors" do + TopicTimer.any_instance.stubs(:enqueue_typed_job).raises(StandardError).then.returns(true) + expect { subject.execute }.not_to raise_error + end end