mirror of
https://github.com/internetee/registry.git
synced 2025-07-27 04:58:29 +02:00
fix: handle HTTPClient::KeepAliveDisconnected in OrgRegistrantPhoneCheckerJob
This commit implements a reliable connection error handling solution for the Company Register API integration. The job previously failed when connection errors occurred without proper recovery mechanisms. The implementation: Adds a lightweight Retryable module with configurable retry logic Implements smart caching of API responses (1 day expiration) Handles common network errors like KeepAliveDisconnected and timeouts Provides a fallback mechanism when all retry attempts fail Ensures test reliability with cache-skipping in test environment Testing: Added specific tests for both recovery and fallback scenarios Verified cache behavior in production and test environments Resolves connection errors observed in production logs without adding unnecessary complexity to the codebase.
This commit is contained in:
parent
832ebff533
commit
a11c0fca2d
4 changed files with 214 additions and 7 deletions
|
@ -1,8 +1,21 @@
|
||||||
class OrgRegistrantPhoneCheckerJob < ApplicationJob
|
class OrgRegistrantPhoneCheckerJob < ApplicationJob
|
||||||
queue_as :default
|
queue_as :default
|
||||||
|
|
||||||
|
include Retryable
|
||||||
|
|
||||||
|
# Constants for API error types
|
||||||
|
API_EXCEPTIONS = [
|
||||||
|
HTTPClient::KeepAliveDisconnected,
|
||||||
|
Net::OpenTimeout,
|
||||||
|
Timeout::Error,
|
||||||
|
Savon::HTTPError,
|
||||||
|
Savon::SOAPFault,
|
||||||
|
Wasabi::Resolver::HTTPError
|
||||||
|
].freeze
|
||||||
|
|
||||||
|
CACHE_EXPIRES_IN = 1.day
|
||||||
|
|
||||||
def perform(type: 'bulk', registrant_user_code: nil, spam_delay: 1)
|
def perform(type: 'bulk', registrant_user_code: nil, spam_delay: 1)
|
||||||
puts '??? PERFROMED ???'
|
|
||||||
case type
|
case type
|
||||||
when 'bulk'
|
when 'bulk'
|
||||||
execute_bulk_checker(spam_delay)
|
execute_bulk_checker(spam_delay)
|
||||||
|
@ -14,7 +27,7 @@ class OrgRegistrantPhoneCheckerJob < ApplicationJob
|
||||||
end
|
end
|
||||||
|
|
||||||
def execute_bulk_checker(spam_delay)
|
def execute_bulk_checker(spam_delay)
|
||||||
log("Bulk checker started")
|
log('Bulk checker started')
|
||||||
|
|
||||||
Contact.where(ident_type: 'org', ident_country_code: 'EE').joins(:registrant_domains).each do |registrant_user|
|
Contact.where(ident_type: 'org', ident_country_code: 'EE').joins(:registrant_domains).each do |registrant_user|
|
||||||
is_phone_number_matching = check_the_registrant_phone_number(registrant_user)
|
is_phone_number_matching = check_the_registrant_phone_number(registrant_user)
|
||||||
|
@ -22,15 +35,16 @@ class OrgRegistrantPhoneCheckerJob < ApplicationJob
|
||||||
sleep(spam_delay)
|
sleep(spam_delay)
|
||||||
end
|
end
|
||||||
|
|
||||||
log("Bulk checker finished")
|
log('Bulk checker finished')
|
||||||
end
|
end
|
||||||
|
|
||||||
def execute_single_checker(registrant_user_code)
|
def execute_single_checker(registrant_user_code)
|
||||||
registrant_user = Contact.where(ident_type: 'org', ident_country_code: 'EE').joins(:registrant_domains).find_by(code: registrant_user_code)
|
registrant_user = Contact.where(ident_type: 'org', ident_country_code: 'EE')
|
||||||
|
.joins(:registrant_domains)
|
||||||
|
.find_by(code: registrant_user_code)
|
||||||
return if registrant_user.nil?
|
return if registrant_user.nil?
|
||||||
|
|
||||||
is_phone_number_matching = check_the_registrant_phone_number(registrant_user)
|
is_phone_number_matching = check_the_registrant_phone_number(registrant_user)
|
||||||
|
|
||||||
call_disclosure_action(is_phone_number_matching, registrant_user)
|
call_disclosure_action(is_phone_number_matching, registrant_user)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -74,7 +88,27 @@ class OrgRegistrantPhoneCheckerJob < ApplicationJob
|
||||||
end
|
end
|
||||||
|
|
||||||
def fetch_phone_number_from_company_register(company_code)
|
def fetch_phone_number_from_company_register(company_code)
|
||||||
|
cache_key = "company_register:#{company_code}:phone_numbers"
|
||||||
|
|
||||||
|
# Skip cache in test environment if environment variable is set
|
||||||
|
return fetch_from_company_register(company_code) if Rails.env.test? && ENV['SKIP_COMPANY_REGISTER_CACHE']
|
||||||
|
|
||||||
|
# Try to get data from cache
|
||||||
|
Rails.cache.fetch(cache_key, expires_in: CACHE_EXPIRES_IN) do
|
||||||
|
fetch_from_company_register(company_code)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def fetch_from_company_register(company_code)
|
||||||
|
# If not in cache, request API with retries
|
||||||
|
with_retry(
|
||||||
|
exceptions: API_EXCEPTIONS,
|
||||||
|
logger: Rails.logger,
|
||||||
|
fallback: -> { log("Failed to get data for company #{company_code}, returning empty array"); [] }
|
||||||
|
) do
|
||||||
data = company_register.company_details(registration_number: company_code.to_s)
|
data = company_register.company_details(registration_number: company_code.to_s)
|
||||||
|
log("Successfully retrieved data for company #{company_code}")
|
||||||
data[0].phone_numbers
|
data[0].phone_numbers
|
||||||
end
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
39
app/lib/retryable.rb
Normal file
39
app/lib/retryable.rb
Normal file
|
@ -0,0 +1,39 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
# Module for retrying operations with external APIs
|
||||||
|
module Retryable
|
||||||
|
# Executes a code block with a specified number of retry attempts in case of specific errors
|
||||||
|
# @param max_attempts [Integer] maximum number of attempts (defaults to 3)
|
||||||
|
# @param retry_delay [Integer] delay between attempts in seconds (defaults to 2)
|
||||||
|
# @param exceptions [Array<Class>] exception classes to catch (defaults to all exceptions)
|
||||||
|
# @param logger [Object] logger object (must support info, warn, error methods)
|
||||||
|
# @param fallback [Proc] code block executed if all attempts fail
|
||||||
|
# @return [Object] result of the block execution or fallback
|
||||||
|
def with_retry(
|
||||||
|
max_attempts: 3,
|
||||||
|
retry_delay: 2,
|
||||||
|
exceptions: [StandardError],
|
||||||
|
logger: Rails.logger,
|
||||||
|
fallback: -> { [] }
|
||||||
|
)
|
||||||
|
attempts = 0
|
||||||
|
|
||||||
|
retry_attempt = lambda do
|
||||||
|
attempts += 1
|
||||||
|
yield
|
||||||
|
rescue *exceptions => e
|
||||||
|
logger.warn("Attempt #{attempts}/#{max_attempts} failed with error: #{e.class} - #{e.message}")
|
||||||
|
|
||||||
|
if attempts < max_attempts
|
||||||
|
logger.info("Retrying in #{retry_delay} seconds...")
|
||||||
|
sleep retry_delay
|
||||||
|
retry_attempt.call
|
||||||
|
else
|
||||||
|
logger.error("All attempts exhausted. Last error: #{e.class} - #{e.message}")
|
||||||
|
fallback.call
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
retry_attempt.call
|
||||||
|
end
|
||||||
|
end
|
|
@ -12,6 +12,17 @@ class OrgRegistrantPhoneCheckerJobTest < ActiveSupport::TestCase
|
||||||
ident_country_code: 'EE',
|
ident_country_code: 'EE',
|
||||||
ident: '12345678'
|
ident: '12345678'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Set environment variable to skip cache in tests
|
||||||
|
ENV['SKIP_COMPANY_REGISTER_CACHE'] = 'true'
|
||||||
|
|
||||||
|
# Clear cache before each test to avoid interference
|
||||||
|
Rails.cache.clear if defined?(Rails.cache)
|
||||||
|
end
|
||||||
|
|
||||||
|
teardown do
|
||||||
|
# Reset environment variable after tests
|
||||||
|
ENV['SKIP_COMPANY_REGISTER_CACHE'] = nil
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_bulk_checker_processes_all_ee_org_contacts
|
def test_bulk_checker_processes_all_ee_org_contacts
|
||||||
|
@ -106,4 +117,59 @@ class OrgRegistrantPhoneCheckerJobTest < ActiveSupport::TestCase
|
||||||
|
|
||||||
CompanyRegister::Client.define_singleton_method(:new, original_new_method)
|
CompanyRegister::Client.define_singleton_method(:new, original_new_method)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Test that successfully retries after a connection error
|
||||||
|
def test_retries_and_recovers_from_connection_error
|
||||||
|
call_count = 0
|
||||||
|
|
||||||
|
original_new_method = CompanyRegister::Client.method(:new)
|
||||||
|
CompanyRegister::Client.define_singleton_method(:new) do
|
||||||
|
object = original_new_method.call
|
||||||
|
def object.company_details(registration_number:)
|
||||||
|
parent_call_count = CompanyRegister::Client.class_variable_get(:@@test_call_count)
|
||||||
|
parent_call_count += 1
|
||||||
|
CompanyRegister::Client.class_variable_set(:@@test_call_count, parent_call_count)
|
||||||
|
|
||||||
|
if parent_call_count == 1
|
||||||
|
raise HTTPClient::KeepAliveDisconnected, "Connection error"
|
||||||
|
end
|
||||||
|
|
||||||
|
[OpenStruct.new(phone_numbers: ['+372.555666777'])]
|
||||||
|
end
|
||||||
|
object
|
||||||
|
end
|
||||||
|
|
||||||
|
CompanyRegister::Client.class_variable_set(:@@test_call_count, 0)
|
||||||
|
|
||||||
|
@contact.disclosed_attributes = []
|
||||||
|
@contact.save!
|
||||||
|
OrgRegistrantPhoneCheckerJob.perform_now(type: 'single', registrant_user_code: @contact.code)
|
||||||
|
@contact.reload
|
||||||
|
|
||||||
|
assert @contact.disclosed_attributes.include?('phone')
|
||||||
|
assert_operator CompanyRegister::Client.class_variable_get(:@@test_call_count), :>=, 2
|
||||||
|
|
||||||
|
CompanyRegister::Client.remove_class_variable(:@@test_call_count)
|
||||||
|
CompanyRegister::Client.define_singleton_method(:new, original_new_method)
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_returns_empty_array_when_max_retries_exceeded
|
||||||
|
original_new_method = CompanyRegister::Client.method(:new)
|
||||||
|
CompanyRegister::Client.define_singleton_method(:new) do
|
||||||
|
object = original_new_method.call
|
||||||
|
def object.company_details(registration_number:)
|
||||||
|
raise HTTPClient::KeepAliveDisconnected, "Persistent connection error"
|
||||||
|
end
|
||||||
|
object
|
||||||
|
end
|
||||||
|
|
||||||
|
@contact.disclosed_attributes = ['phone']
|
||||||
|
@contact.save!
|
||||||
|
OrgRegistrantPhoneCheckerJob.perform_now(type: 'single', registrant_user_code: @contact.code)
|
||||||
|
@contact.reload
|
||||||
|
|
||||||
|
assert_not @contact.disclosed_attributes.include?('phone')
|
||||||
|
|
||||||
|
CompanyRegister::Client.define_singleton_method(:new, original_new_method)
|
||||||
|
end
|
||||||
end
|
end
|
68
test/lib/retryable_test.rb
Normal file
68
test/lib/retryable_test.rb
Normal file
|
@ -0,0 +1,68 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
require 'test_helper'
|
||||||
|
|
||||||
|
class RetryableTest < ActiveSupport::TestCase
|
||||||
|
class TestClass
|
||||||
|
include Retryable
|
||||||
|
|
||||||
|
attr_reader :call_count
|
||||||
|
|
||||||
|
def initialize
|
||||||
|
@call_count = 0
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_retriable_method(should_fail: false, fail_count: 2)
|
||||||
|
with_retry(
|
||||||
|
max_attempts: 3,
|
||||||
|
retry_delay: 0.1,
|
||||||
|
logger: Rails.logger
|
||||||
|
) do
|
||||||
|
@call_count += 1
|
||||||
|
|
||||||
|
if should_fail && @call_count <= fail_count
|
||||||
|
raise StandardError, "Тестовая ошибка #{@call_count}"
|
||||||
|
end
|
||||||
|
|
||||||
|
'success'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_retriable_with_fallback(should_fail: true)
|
||||||
|
with_retry(
|
||||||
|
max_attempts: 2,
|
||||||
|
retry_delay: 0.1,
|
||||||
|
logger: Rails.logger,
|
||||||
|
fallback: -> { 'fallback' }
|
||||||
|
) do
|
||||||
|
@call_count += 1
|
||||||
|
raise StandardError, 'Постоянная ошибка' if should_fail
|
||||||
|
'success'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test 'should retry specified number of times and succeed' do
|
||||||
|
test_object = TestClass.new
|
||||||
|
result = test_object.test_retriable_method(should_fail: true, fail_count: 2)
|
||||||
|
|
||||||
|
assert_equal 3, test_object.call_count
|
||||||
|
assert_equal 'success', result
|
||||||
|
end
|
||||||
|
|
||||||
|
test 'should succeed on first try if no errors' do
|
||||||
|
test_object = TestClass.new
|
||||||
|
result = test_object.test_retriable_method(should_fail: false)
|
||||||
|
|
||||||
|
assert_equal 1, test_object.call_count
|
||||||
|
assert_equal 'success', result
|
||||||
|
end
|
||||||
|
|
||||||
|
test 'should use fallback when all retries fail' do
|
||||||
|
test_object = TestClass.new
|
||||||
|
result = test_object.test_retriable_with_fallback(should_fail: true)
|
||||||
|
|
||||||
|
assert_equal 2, test_object.call_count
|
||||||
|
assert_equal 'fallback', result
|
||||||
|
end
|
||||||
|
end
|
Loading…
Add table
Add a link
Reference in a new issue