Skip to content

Commit

Permalink
Only report Notifier connection errors once after they happen 3 conse…
Browse files Browse the repository at this point in the history
…cutive times
  • Loading branch information
bensheldon committed Aug 6, 2022
1 parent 61ba19a commit 0f9414a
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
21 changes: 20 additions & 1 deletion lib/good_job/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class Notifier
PG::UnableToSend
PG::Error
].freeze
CONNECTION_ERRORS_REPORTING_THRESHOLD = 3

# @!attribute [r] instances
# @!scope class
Expand Down Expand Up @@ -70,6 +71,8 @@ def self.notify(message)
def initialize(*recipients)
@recipients = Concurrent::Array.new(recipients)
@listening = Concurrent::AtomicBoolean.new(false)
@connection_errors_count = Concurrent::AtomicFixnum.new(0)
@connection_errors_reported = Concurrent::AtomicBoolean.new(false)

self.class.instances << self

Expand Down Expand Up @@ -128,7 +131,6 @@ def restart(timeout: -1)
# @return [void]
def listen_observer(_time, _result, thread_error)
if thread_error
GoodJob._on_thread_error(thread_error)
ActiveSupport::Notifications.instrument("notifier_notify_error.good_job", { error: thread_error })

connection_error = CONNECTION_ERRORS.any? do |error_string|
Expand All @@ -137,6 +139,16 @@ def listen_observer(_time, _result, thread_error)

thread_error.is_a? error_class
end

if connection_error
@connection_errors_count.increment
if @connection_errors_reported.false? && @connection_errors_count.value >= CONNECTION_ERRORS_REPORTING_THRESHOLD
GoodJob._on_thread_error(thread_error)
@connection_errors_reported.make_true
end
else
GoodJob._on_thread_error(thread_error)
end
end

return if shutdown?
Expand Down Expand Up @@ -175,6 +187,8 @@ def listen(delay: 0)
target.send(method_name, parsed_payload)
end
end

reset_connection_errors
end
end
end
Expand Down Expand Up @@ -223,5 +237,10 @@ def wait_for_notify
sleep WAIT_INTERVAL
end
end

def reset_connection_errors
@connection_errors_count.value = 0
@connection_errors_reported.make_false
end
end
end
19 changes: 19 additions & 0 deletions spec/lib/good_job/notifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,25 @@

notifier.shutdown
end

it 'raises exception to GoodJob.on_thread_error when there is a connection error' do
stub_const('ExpectedError', Class.new(ActiveRecord::ConnectionNotEstablished))
stub_const('GoodJob::Notifier::RECONNECT_INTERVAL', 0.1)
on_thread_error = instance_double(Proc, call: nil)
allow(GoodJob).to receive(:on_thread_error).and_return(on_thread_error)
allow(JSON).to receive(:parse).and_raise ExpectedError

notifier = described_class.new
sleep_until(max: 5, increments_of: 0.5) { notifier.listening? }

5.times do
described_class.notify(true)
sleep 0.1
end
wait_until(max: 5, increments_of: 0.5) { expect(on_thread_error).to have_received(:call).at_least(:once).with instance_of(ExpectedError) }

notifier.shutdown
end
end

describe 'Process tracking' do
Expand Down

0 comments on commit 0f9414a

Please sign in to comment.