From f95cefd09a66b989be1d29e788f482d3d99929d4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 6 Apr 2016 14:59:48 +0800 Subject: [PATCH] FEATURE: Add POP3 timeout error only after 3 failures in a row. --- app/jobs/scheduled/poll_mailbox.rb | 23 +++++++++++++++---- spec/jobs/poll_mailbox_spec.rb | 37 ++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 2c52898a972..af206493014 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/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! - AdminDashboardData.add_problem_message('dashboard.poll_pop3_timeout', SiteSetting.pop3_polling_period_mins.minutes + 5.minutes) - Discourse.handle_job_exception(e, error_context(@args, "Connecting to '#{SiteSetting.pop3_polling_host}' for polling emails.")) + count = $redis.incr(POLL_MAILBOX_TIMEOUT_ERROR_KEY).to_i + $redis.expire(POLL_MAILBOX_TIMEOUT_ERROR_KEY, 300) if count == 1 + + 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 diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index a695be6145c..488674a93f4 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/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 - Net::POP3.any_instance.expects(:start).raises(Net::POPAuthenticationError.new) - Discourse.expects(:handle_job_exception) - poller.poll_pop3 + context "pop errors" do + let(:user) { Fabricate(:user) } + + 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