From 6b9f4d08ca37c5bd6fd4dd154e3929f7335904b7 Mon Sep 17 00:00:00 2001 From: Pranav Raj S Date: Thu, 2 Jun 2022 11:38:58 +0530 Subject: [PATCH] fix: Disable marking IMAP connection as invalid for Standard Errors (#4764) --- app/jobs/inboxes/fetch_imap_emails_job.rb | 19 ++++++++++++------- lib/chatwoot_exception_tracker.rb | 7 +++++-- spec/lib/chatwoot_exception_tracker_spec.rb | 5 +++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/app/jobs/inboxes/fetch_imap_emails_job.rb b/app/jobs/inboxes/fetch_imap_emails_job.rb index eab5e4701..2b77d068c 100644 --- a/app/jobs/inboxes/fetch_imap_emails_job.rb +++ b/app/jobs/inboxes/fetch_imap_emails_job.rb @@ -6,13 +6,12 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob def perform(channel) return unless should_fetch_email?(channel) - process_mail_for_channel(channel) + fetch_mail_for_channel(channel) # clearing old failures like timeouts since the mail is now successfully processed channel.reauthorized! rescue Errno::ECONNREFUSED, Net::OpenTimeout, Net::IMAP::NoResponseError channel.authorization_error! rescue StandardError => e - channel.authorization_error! ChatwootExceptionTracker.new(e, account: channel.account).capture_exception end @@ -22,7 +21,7 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob channel.imap_enabled? && !channel.reauthorization_required? end - def process_mail_for_channel(channel) + def fetch_mail_for_channel(channel) # TODO: rather than setting this as default method for all mail objects, lets if can do new mail object # using Mail.retriever_method.new(params) Mail.defaults do @@ -36,12 +35,18 @@ class Inboxes::FetchImapEmailsJob < ApplicationJob new_mails = false Mail.find(what: :last, count: 10, order: :desc).each do |inbound_mail| - if inbound_mail.date.utc >= channel.imap_inbox_synced_at - Imap::ImapMailbox.new.process(inbound_mail, channel) - new_mails = true - end + next unless inbound_mail.date.utc >= channel.imap_inbox_synced_at + + process_mail(inbound_mail, channel) + new_mails = true end channel.update(imap_inbox_synced_at: Time.now.utc) if new_mails end + + def process_mail(inbound_mail, channel) + Imap::ImapMailbox.new.process(inbound_mail, channel) + rescue StandardError => e + ChatwootExceptionTracker.new(e, account: channel.account).capture_exception + end end diff --git a/lib/chatwoot_exception_tracker.rb b/lib/chatwoot_exception_tracker.rb index 5e08d2dad..e525f3e89 100644 --- a/lib/chatwoot_exception_tracker.rb +++ b/lib/chatwoot_exception_tracker.rb @@ -12,8 +12,11 @@ class ChatwootExceptionTracker end def capture_exception - capture_exception_with_sentry if ENV['SENTRY_DSN'].present? - # Implement other providers like honeybadger, rollbar etc in future + if ENV['SENTRY_DSN'].present? + capture_exception_with_sentry + else + Rails.logger.error @exception + end end private diff --git a/spec/lib/chatwoot_exception_tracker_spec.rb b/spec/lib/chatwoot_exception_tracker_spec.rb index 5e8164d6d..d0d1b37a3 100644 --- a/spec/lib/chatwoot_exception_tracker_spec.rb +++ b/spec/lib/chatwoot_exception_tracker_spec.rb @@ -1,8 +1,9 @@ require 'rails_helper' describe ChatwootExceptionTracker do - it 'returns nil if no tracker is configured' do - expect(described_class.new('random').capture_exception).to eq(nil) + it 'use rails logger if no tracker is configured' do + expect(Rails.logger).to receive(:error).with('random') + described_class.new('random').capture_exception end context 'with sentry DSN' do