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.

<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
This commit is contained in:
Martin Brennan 2021-07-08 12:49:58 +10:00 committed by GitHub
parent 3f23dda73b
commit c6f2459cc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 4 deletions

View File

@ -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
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

View File

@ -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