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.
This commit is contained in:
Sam 2022-08-03 14:28:46 +10:00 committed by GitHub
parent 035f5d7a5d
commit 3b42e69174
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 5 deletions

View File

@ -175,9 +175,13 @@ module Discourse
context ||= {} context ||= {}
parent_logger ||= Sidekiq parent_logger ||= Sidekiq
job = context.dig(:job, "class") job = context[:job]
if job
job_exception_stats[job] += 1 if Hash === job
job_class = job["class"]
if job_class
job_exception_stats[job_class] += 1
end
end end
cm = RailsMultisite::ConnectionManagement cm = RailsMultisite::ConnectionManagement

View File

@ -342,6 +342,9 @@ RSpec.describe Discourse do
describe "#job_exception_stats" do describe "#job_exception_stats" do
class FakeTestError < StandardError
end
before do before do
Discourse.reset_job_exception_stats! Discourse.reset_job_exception_stats!
end end
@ -350,6 +353,12 @@ RSpec.describe Discourse do
Discourse.reset_job_exception_stats! Discourse.reset_job_exception_stats!
end 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 it "should collect job exception stats" do
# see MiniScheduler Manager which reports it like this # see MiniScheduler Manager which reports it like this
@ -361,7 +370,7 @@ RSpec.describe Discourse do
# re-raised unconditionally in test env # re-raised unconditionally in test env
2.times do 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 end
exception_context = { exception_context = {
@ -369,7 +378,7 @@ RSpec.describe Discourse do
job: { "class" => Jobs::PollMailbox } 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({ expect(Discourse.job_exception_stats).to eq({
Jobs::PollMailbox => 1, Jobs::PollMailbox => 1,