From 892d71eca5f059303b5a690efc49aca24c054657 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Thu, 26 Feb 2026 14:09:01 +0530 Subject: [PATCH] review changes --- app/jobs/inboxes/fetch_imap_emails_job.rb | 8 +-- app/models/channel/email.rb | 49 +--------------- app/models/concerns/backoffable.rb | 70 +++++++++++++++++++++++ app/models/concerns/reauthorizable.rb | 2 + config/installation_config.yml | 14 ++--- lib/redis/redis_keys.rb | 6 +- 6 files changed, 88 insertions(+), 61 deletions(-) create mode 100644 app/models/concerns/backoffable.rb diff --git a/app/jobs/inboxes/fetch_imap_emails_job.rb b/app/jobs/inboxes/fetch_imap_emails_job.rb index 75004d2ce..74368c830 100644 --- a/app/jobs/inboxes/fetch_imap_emails_job.rb +++ b/app/jobs/inboxes/fetch_imap_emails_job.rb @@ -16,13 +16,13 @@ class Inboxes::FetchImapEmailsJob < MutexApplicationJob def fetch_emails_with_backoff(channel, interval) key = format(::Redis::Alfred::EMAIL_MESSAGE_MUTEX, inbox_id: channel.inbox.id) with_lock(key, 5.minutes) { process_email_for_channel(channel, interval) } - channel.clear_imap_backoff! + channel.clear_backoff! rescue Imap::AuthenticationError => e - Rails.logger.error "Authentication error for email channel - #{channel.inbox.id} : #{e.message}" + Rails.logger.error "#{channel.backoff_log_identifier} authentication error : #{e.message}" channel.authorization_error! rescue *ExceptionList::IMAP_TRANSIENT_EXCEPTIONS => e - Rails.logger.error "Error for email channel - #{channel.inbox.id} : #{e.message}" - channel.apply_imap_backoff! + Rails.logger.error "#{channel.backoff_log_identifier} transient error : #{e.message}" + channel.apply_backoff! rescue LockAcquisitionError Rails.logger.error "Lock failed for #{channel.inbox.id}" end diff --git a/app/models/channel/email.rb b/app/models/channel/email.rb index 416b3915c..1091b17d8 100644 --- a/app/models/channel/email.rb +++ b/app/models/channel/email.rb @@ -72,31 +72,8 @@ class Channel::Email < ApplicationRecord imap_enabled && imap_address == 'imap.gmail.com' end - def imap_retry_count - ::Redis::Alfred.get(imap_retry_count_key).to_i - end - - def in_backoff? - val = ::Redis::Alfred.get(imap_retry_after_key) - val.present? && Time.zone.at(val.to_f) > Time.current - end - - def apply_imap_backoff! - new_count = imap_retry_count + 1 - max_retries = imap_backoff_max_retries - - if new_count > max_retries - Rails.logger.warn "Error for email channel - #{inbox.id} exhausted backoff (#{new_count} failures), prompting reauthorization" - clear_imap_backoff! - prompt_reauthorization! - else - schedule_imap_retry(new_count, max_retries) - end - end - - def clear_imap_backoff! - ::Redis::Alfred.delete(imap_retry_count_key) - ::Redis::Alfred.delete(imap_retry_after_key) + def backoff_log_identifier + "Error for email channel - #{inbox.id}" end private @@ -104,26 +81,4 @@ class Channel::Email < ApplicationRecord def ensure_forward_to_email self.forward_to_email ||= "#{SecureRandom.hex}@#{account.inbound_email_domain}" end - - def imap_backoff_max_retries - max_interval = GlobalConfigService.load('IMAP_BACKOFF_MAX_INTERVAL_MINUTES', 5).to_i - max_count = GlobalConfigService.load('IMAP_BACKOFF_MAX_INTERVAL_COUNT', 10).to_i - (max_interval - 1) + max_count - end - - def schedule_imap_retry(new_count, max_retries) - max_interval = GlobalConfigService.load('IMAP_BACKOFF_MAX_INTERVAL_MINUTES', 5).to_i - wait_minutes = [new_count, max_interval].min - ::Redis::Alfred.set(imap_retry_count_key, new_count.to_s) - ::Redis::Alfred.set(imap_retry_after_key, wait_minutes.minutes.from_now.to_f.to_s) - Rails.logger.warn "Error for email channel - #{inbox.id} backoff retry #{new_count}/#{max_retries}, next attempt in #{wait_minutes}m" - end - - def imap_retry_count_key - format(::Redis::Alfred::IMAP_BACKOFF_RETRY_COUNT, channel_id: id) - end - - def imap_retry_after_key - format(::Redis::Alfred::IMAP_BACKOFF_RETRY_AFTER, channel_id: id) - end end diff --git a/app/models/concerns/backoffable.rb b/app/models/concerns/backoffable.rb new file mode 100644 index 000000000..60b6ac98c --- /dev/null +++ b/app/models/concerns/backoffable.rb @@ -0,0 +1,70 @@ +# Backoffable provides transient-error retry backoff for models that depend on external services. +# +# When a transient error occurs (network hiccup, SSL failure, etc.) call apply_backoff!. +# The wait time ramps from 1 minute up to BACKOFF_MAX_INTERVAL_MINUTES, then holds at that +# ceiling for BACKOFF_MAX_INTERVAL_COUNT more attempts before calling prompt_reauthorization!. +# +# Call clear_backoff! after a successful operation to reset the counter. + +module Backoffable + extend ActiveSupport::Concern + + def backoff_log_identifier + inbox_id = respond_to?(:inbox) && inbox&.id + inbox_id ? "#{self.class.name} - #{inbox_id}" : "#{self.class.name}##{id}" + end + + def backoff_retry_count + ::Redis::Alfred.get(backoff_retry_count_key).to_i + end + + def in_backoff? + val = ::Redis::Alfred.get(backoff_retry_after_key) + val.present? && Time.zone.at(val.to_f) > Time.current + end + + def apply_backoff! + new_count = backoff_retry_count + 1 + max_interval, max_retries = backoff_limits + + if new_count > max_retries + exhaust_backoff(new_count) + else + schedule_backoff_retry(new_count, max_interval, max_retries) + end + end + + def clear_backoff! + ::Redis::Alfred.delete(backoff_retry_count_key) + ::Redis::Alfred.delete(backoff_retry_after_key) + end + + private + + def backoff_limits + max_interval = GlobalConfigService.load('BACKOFF_MAX_INTERVAL_MINUTES', 5).to_i + max_count = GlobalConfigService.load('BACKOFF_MAX_INTERVAL_COUNT', 10).to_i + [max_interval, (max_interval - 1) + max_count] + end + + def exhaust_backoff(new_count) + Rails.logger.warn "#{backoff_log_identifier} backoff exhausted (#{new_count} failures), prompting reauthorization" + clear_backoff! + prompt_reauthorization! + end + + def schedule_backoff_retry(new_count, max_interval, max_retries) + wait_minutes = [new_count, max_interval].min + ::Redis::Alfred.set(backoff_retry_count_key, new_count.to_s, ex: 24.hours) + ::Redis::Alfred.set(backoff_retry_after_key, wait_minutes.minutes.from_now.to_f.to_s, ex: 24.hours) + Rails.logger.warn "#{backoff_log_identifier} backoff retry #{new_count}/#{max_retries}, next attempt in #{wait_minutes}m" + end + + def backoff_retry_count_key + format(::Redis::Alfred::BACKOFF_RETRY_COUNT, obj_type: self.class.table_name.singularize, obj_id: id) + end + + def backoff_retry_after_key + format(::Redis::Alfred::BACKOFF_RETRY_AFTER, obj_type: self.class.table_name.singularize, obj_id: id) + end +end diff --git a/app/models/concerns/reauthorizable.rb b/app/models/concerns/reauthorizable.rb index 7a09f6436..1eecd29f4 100644 --- a/app/models/concerns/reauthorizable.rb +++ b/app/models/concerns/reauthorizable.rb @@ -13,6 +13,8 @@ module Reauthorizable extend ActiveSupport::Concern + include Backoffable + AUTHORIZATION_ERROR_THRESHOLD = 2 # model attribute diff --git a/config/installation_config.yml b/config/installation_config.yml index 754a64f32..0d22de6fc 100644 --- a/config/installation_config.yml +++ b/config/installation_config.yml @@ -96,18 +96,18 @@ locked: false # ------- End of Account Related Config ------- # -# ------- IMAP Backoff Related Config ------- # -- name: IMAP_BACKOFF_MAX_INTERVAL_MINUTES - display_title: 'IMAP Backoff Max Interval (minutes)' - description: 'Maximum wait time in minutes between IMAP retry attempts before the backoff plateaus' +# ------- Transient Error Backoff Config ------- # +- name: BACKOFF_MAX_INTERVAL_MINUTES + display_title: 'Backoff Max Interval (minutes)' + description: 'Maximum wait time in minutes between retry attempts before the backoff plateaus' value: 5 locked: false -- name: IMAP_BACKOFF_MAX_INTERVAL_COUNT - display_title: 'IMAP Backoff Max Retry Count' +- name: BACKOFF_MAX_INTERVAL_COUNT + display_title: 'Backoff Max Retry Count' description: 'Number of additional retries at the maximum interval before prompting reauthorization' value: 10 locked: false -# ------- End of IMAP Backoff Related Config ------- # +# ------- End of Transient Error Backoff Config ------- # # ------- Email Related Config ------- # - name: MAILER_INBOUND_EMAIL_DOMAIN diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index 5663822ca..fa9eeb31e 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -53,7 +53,7 @@ module Redis::RedisKeys ## Account Email Rate Limiting ACCOUNT_OUTBOUND_EMAIL_COUNT_KEY = 'OUTBOUND_EMAIL_COUNT::%d::%s'.freeze - ## IMAP Backoff / Fetch Tracking (channel_email) - IMAP_BACKOFF_RETRY_COUNT = 'IMAP_BACKOFF:channel_email:%d:retry_count'.freeze - IMAP_BACKOFF_RETRY_AFTER = 'IMAP_BACKOFF:channel_email:%d:retry_after'.freeze + ## Transient Error Backoff + BACKOFF_RETRY_COUNT = 'BACKOFF:%s:%d:retry_count'.freeze + BACKOFF_RETRY_AFTER = 'BACKOFF:%s:%d:retry_after'.freeze end