Merge pull request #4145 from tgxworld/better_error_handling_of_email_polling

FEATURE: Add POP3 timeout error only after 3 failures in a row.

app/jobs/scheduled/poll_mailbox.rb

@@ -105,6 +105,8 @@ module Jobs
       client_message
     end
 
+    POLL_MAILBOX_TIMEOUT_ERROR_KEY = "poll_mailbox_timeout_error_key".freeze
+
     def poll_pop3
       pop3 = Net::POP3.new(SiteSetting.pop3_polling_host, SiteSetting.pop3_polling_port)
       pop3.enable_ssl if SiteSetting.pop3_polling_ssl
@@ -115,12 +117,18 @@ module Jobs
         end
       end
     rescue Net::OpenTimeout => e
-      mark_as_errored!
+      count = $redis.incr(POLL_MAILBOX_TIMEOUT_ERROR_KEY).to_i
-      AdminDashboardData.add_problem_message('dashboard.poll_pop3_timeout', SiteSetting.pop3_polling_period_mins.minutes + 5.minutes)
+      $redis.expire(POLL_MAILBOX_TIMEOUT_ERROR_KEY, 300) if count == 1
-      Discourse.handle_job_exception(e, error_context(@args, "Connecting to '#{SiteSetting.pop3_polling_host}' for polling emails."))
+
+      if count > 3
+        $redis.del(POLL_MAILBOX_TIMEOUT_ERROR_KEY)
+        mark_as_errored!
+        add_admin_dashboard_problem_message('dashboard.poll_pop3_timeout')
+        Discourse.handle_job_exception(e, error_context(@args, "Connecting to '#{SiteSetting.pop3_polling_host}' for polling emails."))
+      end
     rescue Net::POPAuthenticationError => e
       mark_as_errored!
-      AdminDashboardData.add_problem_message('dashboard.poll_pop3_auth_error', SiteSetting.pop3_polling_period_mins.minutes + 5.minutes)
+      add_admin_dashboard_problem_message('dashboard.poll_pop3_auth_error')
       Discourse.handle_job_exception(e, error_context(@args, "Signing in to poll incoming emails."))
     end
 
@@ -148,5 +156,12 @@ module Jobs
       end
     end
 
+    def add_admin_dashboard_problem_message(i18n_key)
+      AdminDashboardData.add_problem_message(
+        i18n_key,
+        SiteSetting.pop3_polling_period_mins.minutes + 5.minutes
+      )
+    end
+
   end
 end

spec/jobs/poll_mailbox_spec.rb

@@ -23,10 +23,39 @@ describe Jobs::PollMailbox do
 
   describe ".poll_pop3" do
 
-    it "logs an error on pop authentication error" do
+    context "pop errors" do
-      Net::POP3.any_instance.expects(:start).raises(Net::POPAuthenticationError.new)
+      let(:user) { Fabricate(:user) }
-      Discourse.expects(:handle_job_exception)
+
-      poller.poll_pop3
+      before do
+        Discourse.expects(:handle_job_exception).at_least_once
+      end
+
+      after do
+        $redis.flushall
+      end
+
+      it "add an admin dashboard message on pop authentication error" do
+        Net::POP3.any_instance.expects(:start)
+          .raises(Net::POPAuthenticationError.new).at_least_once
+
+        poller.poll_pop3
+
+        i18n_key = 'dashboard.poll_pop3_auth_error'
+
+        expect(AdminDashboardData.problem_message_check(i18n_key))
+          .to eq(I18n.t(i18n_key))
+      end
+
+      it "logs an error on pop connection timeout error" do
+        Net::POP3.any_instance.expects(:start).raises(Net::OpenTimeout.new).at_least_once
+
+        4.times { poller.poll_pop3 }
+
+        i18n_key = 'dashboard.poll_pop3_timeout'
+
+        expect(AdminDashboardData.problem_message_check(i18n_key))
+          .to eq(I18n.t(i18n_key))
+      end
     end
 
     it "calls enable_ssl when the setting is enabled" do