WhoisSyncJob : better error handling

app/jobs/whois_sync_job.rb

@@ -1,25 +1,28 @@
 class WhoisSyncJob < ApplicationJob
   queue_as :default
 
+  rescue_from StandardError do |exception|
+    check_logger.log(:standard_error, exception) if check.present?
+    raise # rubocop:disable Style/SignalException
+  end
+
   rescue_from ActiveRecord::RecordNotFound do; end
 
+  # parser error are already logged
+  rescue_from Whois::Error do; end
+
   attr_reader :check
 
   def perform(check_id)
-    @check = Check.find(check_id)
-    check.update_attribute(:last_run_at, Time.now)
+    prepare_check(check_id)
 
-    response = Whois.ask(check.domain)
+    response = Whois.ask(check.domain, logger: check_logger)
     return unless response.valid?
 
     update_from_response(response)
-
-    check.save!
   rescue Whois::DomainNotFoundError
     check.active = false
     check.save!
-  rescue Whois::ParserError # rubocop:disable Lint/HandleExceptions
-    # already logged
   end
 
   def update_from_response(response)
@@ -27,5 +30,19 @@ class WhoisSyncJob < ApplicationJob
     check.domain_updated_at = response.updated_at
     check.domain_expires_at = response.expire_at
     check.last_success_at = Time.now
+
+    check.save!
+  end
+
+  private
+
+  def prepare_check(check_id)
+    @check = Check.find(check_id)
+    check.update_attribute(:last_run_at, Time.now)
+  end
+
+  # logger is a reserved ActiveJob method
+  def check_logger
+    @check_logger ||= CheckLogger.new(check)
   end
 end

app/services/check_logger.rb

@@ -11,7 +11,7 @@ class CheckLogger
       log_command_result(message)
     when :parsed_response
       log_parsed_response(message)
-    when :parser_error, :service_error
+    when :parser_error, :service_error, :standard_error
       log_error(message)
     end
   end

app/services/whois/errors.rb

@@ -1,10 +1,10 @@
 module Whois
-  class WhoisError < StandardError; end
+  class Error < StandardError; end
 
-  class WhoisCommandError < WhoisError; end
-  class UnsupportedDomainError < WhoisError; end
-  class DomainNotFoundError < WhoisError; end
-  class ParserError < WhoisError; end
+  class WhoisCommandError < Error; end
+  class UnsupportedDomainError < Error; end
+  class DomainNotFoundError < Error; end
+  class ParserError < Error; end
 
   class FieldNotFoundError < ParserError; end
   class MissingDateFormatError < ParserError; end

app/services/whois/parser/base.rb

@@ -29,7 +29,7 @@ module Whois
         logger.log :parsed_response, response
 
         response
-      rescue StandardError => ex
+      rescue ParserError => ex
         logger.log :parser_error, ex
         raise
       end

test/jobs/whois_sync_job_test.rb

@@ -6,7 +6,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     check = create(:check, :nil_dates, domain: domain)
 
     mock_system_command("whois", domain, stdout: whois_response(domain)) do
-      WhoisSyncJob.new.perform(check.id)
+      WhoisSyncJob.perform_now(check.id)
     end
 
     check.reload
@@ -24,7 +24,7 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     original_updated_at = check.updated_at
 
     mock_system_command("whois", "domain.fr", stdout: "not a response") do
-      WhoisSyncJob.new.perform(check.id)
+      WhoisSyncJob.perform_now(check.id)
     end
 
     check.reload
@@ -35,12 +35,32 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     assert check.active?
   end
 
-  test "Disable check when whois responds domain not found" do
+  test "should ignore not found (removed) checks" do
+    assert_nothing_raised do
+      WhoisSyncJob.perform_now("9999999")
+    end
+  end
+
+  test "should log and re-raise StandardError" do
+    check = create(:check)
+
+    assert_raise StandardError do
+      Whois.stub :ask, nil do
+        WhoisSyncJob.perform_now(check.id)
+      end
+    end
+
+    assert_equal 1, check.logs.count
+    assert_match(/undefined method \W+valid\?/, check.logs.last.error)
+    assert check.logs.last.failed?
+  end
+
+  test "disable check when whois responds domain not found" do
     domain = "willneverexist.fr"
     check = create(:check, :nil_dates, domain: domain)
 
     mock_system_command("whois", domain, stdout: whois_response(domain)) do
-      WhoisSyncJob.new.perform(check.id)
+      WhoisSyncJob.perform_now(check.id)
     end
 
     check.reload
@@ -50,6 +70,16 @@ class WhoisSyncJobTest < ActiveJob::TestCase
     assert_nil check.last_success_at
   end
 
+  test "default logger is CheckLogger" do
+    check = create(:check)
+
+    mock_system_command("whois", check.domain) do
+      WhoisSyncJob.perform_now(check.id)
+    end
+
+    assert_equal 1, check.logs.count
+  end
+
   private
 
   def whois_response(domain)

test/services/check_logger_test.rb

@@ -39,7 +39,7 @@ class CheckLoggerTest < ActiveSupport::TestCase
     assert @logger.check_log.failed?
   end
 
-  test "should log a successful parsed command" do
+  test "should log a successful parsed response" do
     response = OpenStruct.new(
       domain: "example.fr",
       extracted: "some data",
@@ -52,6 +52,17 @@ class CheckLoggerTest < ActiveSupport::TestCase
     assert @logger.check_log.succeed?
   end
 
+  test "should log as failed a empty/error parsed response" do
+    response = OpenStruct.new(
+      domain: "example.fr",
+      valid?: false,
+    )
+    @logger.log :parsed_response, response
+
+    assert_equal response.to_json, @logger.check_log.parsed_response
+    assert @logger.check_log.failed?
+  end
+
   test "should log parser error with a backtrace" do
     @logger.log :parser_error, mock_exception
 
@@ -60,6 +71,14 @@ class CheckLoggerTest < ActiveSupport::TestCase
     assert @logger.check_log.failed?
   end
 
+  test "should log standard error with a backtrace" do
+    @logger.log :standard_error, mock_exception
+
+    assert_includes @logger.check_log.error, "my error occured"
+    assert_includes @logger.check_log.error, "minitest.rb"
+    assert @logger.check_log.failed?
+  end
+
   test "should log service error" do
     @logger.log :service_error, mock_exception