From 3b42e69174aaa8b7d930f56bcfd21de044809239 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 3 Aug 2022 14:28:46 +1000 Subject: [PATCH] FIX: avoid usage of dig when looking for job class (#17772) `{a: "a"}.dig(:a, :b)` will result in an exception, since ruby assumes that `"a"` will be another hash it can look up the `:b` key on. --- lib/discourse.rb | 10 +++++++--- spec/lib/discourse_spec.rb | 13 +++++++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/discourse.rb b/lib/discourse.rb index 6891f83789a..5f7334d12e4 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -175,9 +175,13 @@ module Discourse context ||= {} parent_logger ||= Sidekiq - job = context.dig(:job, "class") - if job - job_exception_stats[job] += 1 + job = context[:job] + + if Hash === job + job_class = job["class"] + if job_class + job_exception_stats[job_class] += 1 + end end cm = RailsMultisite::ConnectionManagement diff --git a/spec/lib/discourse_spec.rb b/spec/lib/discourse_spec.rb index 783958939c0..653d6268349 100644 --- a/spec/lib/discourse_spec.rb +++ b/spec/lib/discourse_spec.rb @@ -342,6 +342,9 @@ RSpec.describe Discourse do describe "#job_exception_stats" do + class FakeTestError < StandardError + end + before do Discourse.reset_job_exception_stats! end @@ -350,6 +353,12 @@ RSpec.describe Discourse do Discourse.reset_job_exception_stats! end + it "should not fail on incorrectly shaped hash" do + expect do + Discourse.handle_job_exception(FakeTestError.new, { job: "test" }) + end.to raise_error(FakeTestError) + end + it "should collect job exception stats" do # see MiniScheduler Manager which reports it like this @@ -361,7 +370,7 @@ RSpec.describe Discourse do # re-raised unconditionally in test env 2.times do - expect { Discourse.handle_job_exception(StandardError.new, exception_context) }.to raise_error(StandardError) + expect { Discourse.handle_job_exception(FakeTestError.new, exception_context) }.to raise_error(FakeTestError) end exception_context = { @@ -369,7 +378,7 @@ RSpec.describe Discourse do job: { "class" => Jobs::PollMailbox } } - expect { Discourse.handle_job_exception(StandardError.new, exception_context) }.to raise_error(StandardError) + expect { Discourse.handle_job_exception(FakeTestError.new, exception_context) }.to raise_error(FakeTestError) expect(Discourse.job_exception_stats).to eq({ Jobs::PollMailbox => 1,