diff --git a/app/models/check.rb b/app/models/check.rb index ac4ba08..2afdea0 100644 --- a/app/models/check.rb +++ b/app/models/check.rb @@ -52,6 +52,7 @@ class Check < ApplicationRecord validates :comment, length: { maximum: 255 } validates :vendor, length: { maximum: 255 } + before_save :reset_consecutive_failures after_update :reset_notifications after_save :enqueue_sync @@ -88,6 +89,11 @@ class Check < ApplicationRecord (Date.today - last_success_at.to_date).to_i end + def increment_consecutive_failures! + self.consecutive_failures += 1 + save! + end + private def domain_created_at_past @@ -110,4 +116,11 @@ class Check < ApplicationRecord notifications.each(&:reset!) end + + def reset_consecutive_failures + return unless last_success_at_changed? + return if consecutive_failures_changed? + + self.consecutive_failures = 0 + end end diff --git a/app/services/check_logger.rb b/app/services/check_logger.rb index e04b077..5fe256a 100644 --- a/app/services/check_logger.rb +++ b/app/services/check_logger.rb @@ -1,8 +1,11 @@ class CheckLogger + attr_reader :check attr_reader :check_log def initialize(check) + @check = check @check_log = CheckLog.create!(check: check, status: :pending) + @error_logged = false end def log(event, message) @@ -12,7 +15,13 @@ class CheckLogger when :parsed_response log_parsed_response(message) when :parser_error, :service_error, :standard_error + # avoid multiple logging & wrong incrementation of consecutive failures + # (because a Service exception could be re-raised from a Job) + return if error_logged? + log_error(message) + + @error_logged = true end end @@ -41,7 +50,12 @@ class CheckLogger end def log_error(exception) + check.increment_consecutive_failures! check_log.error = [exception.message, exception.backtrace].join("\n") check_log.failed! end + + def error_logged? + @error_logged + end end diff --git a/test/jobs/ssl_sync_job_test.rb b/test/jobs/ssl_sync_job_test.rb index 30b66a4..44e76ed 100644 --- a/test/jobs/ssl_sync_job_test.rb +++ b/test/jobs/ssl_sync_job_test.rb @@ -22,7 +22,6 @@ class SSLSyncJobTest < ActiveJob::TestCase test "ignore invalid response" do domain = "domain.fr" check = create(:check, :nil_dates, domain: domain) - original_updated_at = check.updated_at mock_system_command("check_http", expected_command_arg(domain), stdout: "not a response") do SSLSyncJob.perform_now(check.id) @@ -32,8 +31,8 @@ class SSLSyncJobTest < ActiveJob::TestCase assert_just_now check.last_run_at assert_nil check.last_success_at - assert_equal original_updated_at, check.updated_at assert check.active? + assert_equal 1, check.consecutive_failures end test "should ignore not found (removed) checks" do @@ -51,9 +50,25 @@ class SSLSyncJobTest < ActiveJob::TestCase end end + check.reload + assert_equal 1, check.logs.count assert_match(/undefined method \W+valid\?/, check.logs.last.error) assert check.logs.last.failed? + assert_equal 1, check.consecutive_failures + end + + test "should reset consecutive failures with a valid response" do + domain = "ssl0.domain.org" + check = create(:check, :nil_dates, domain: domain, consecutive_failures: 1) + + mock_system_command("check_http", expected_command_arg(domain), stdout: ssl_response(domain)) do + SSLSyncJob.perform_now(check.id) + end + + check.reload + + assert_equal 0, check.consecutive_failures end private diff --git a/test/jobs/whois_sync_job_test.rb b/test/jobs/whois_sync_job_test.rb index e96755d..401eba0 100644 --- a/test/jobs/whois_sync_job_test.rb +++ b/test/jobs/whois_sync_job_test.rb @@ -33,6 +33,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase assert_nil check.last_success_at assert_equal original_updated_at, check.updated_at assert check.active? + assert_equal 1, check.consecutive_failures end test "should ignore not found (removed) checks" do @@ -50,9 +51,12 @@ class WhoisSyncJobTest < ActiveJob::TestCase end end + check.reload + assert_equal 1, check.logs.count assert_match(/undefined method \W+valid\?/, check.logs.last.error) assert check.logs.last.failed? + assert_equal 1, check.consecutive_failures end test "disable check when whois responds domain not found" do @@ -68,6 +72,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase refute check.active? assert_just_now check.last_run_at assert_nil check.last_success_at + assert_equal 1, check.consecutive_failures end test "default logger is CheckLogger" do @@ -80,6 +85,19 @@ class WhoisSyncJobTest < ActiveJob::TestCase assert_equal 1, check.logs.count end + test "should reset consecutive failures with a valid response" do + domain = "domain.fr" + check = create(:check, :nil_dates, domain: domain, consecutive_failures: 1) + + mock_system_command("whois", domain, stdout: whois_response(domain)) do + WhoisSyncJob.perform_now(check.id) + end + + check.reload + + assert_equal 0, check.consecutive_failures + end + private def whois_response(domain) diff --git a/test/models/check_test.rb b/test/models/check_test.rb index 74a043a..1fee4e8 100644 --- a/test/models/check_test.rb +++ b/test/models/check_test.rb @@ -87,7 +87,7 @@ class CheckTest < ActiveSupport::TestCase test "in_error? when last check occured a few days ago without error" do check = build(:check, created_at: 3.weeks.ago, - last_success_at: 10.days.ago, last_run_at: 10.days.ago) + last_success_at: 10.days.ago, last_run_at: 10.days.ago) refute check.in_error? end