From 51322a46b3fabab11cfa8f29ac4f386d8227aedf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 8 Aug 2016 22:28:27 +0200
Subject: [PATCH] FEATURE: retry processing incoming emails on rate limit

---
 app/jobs/regular/process_email.rb       | 16 +++++++++++++++
 config/locales/server.en.yml            |  7 -------
 lib/email/processor.rb                  | 14 ++++++-------
 spec/components/email/processor_spec.rb | 27 +++++++++++++++++++++++++
 spec/jobs/process_email_spec.rb         | 12 +++++++++++
 5 files changed, 61 insertions(+), 15 deletions(-)
 create mode 100644 app/jobs/regular/process_email.rb
 create mode 100644 spec/components/email/processor_spec.rb
 create mode 100644 spec/jobs/process_email_spec.rb

diff --git a/app/jobs/regular/process_email.rb b/app/jobs/regular/process_email.rb
new file mode 100644
index 000000000..a01e3bc07
--- /dev/null
+++ b/app/jobs/regular/process_email.rb
@@ -0,0 +1,16 @@
+module Jobs
+
+  class ProcessEmail < Jobs::Base
+    sidekiq_options retry: 3
+
+    def execute(args)
+      Email::Processor.process!(args[:mail], false)
+    end
+
+    sidekiq_retries_exhausted do |msg|
+      Rails.logger.warn("Incoming email could not be processed after 3 retries.\n\n#{msg["args"][:mail]}")
+    end
+
+  end
+
+end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 9fdcd7c71..50188377c 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -2054,13 +2054,6 @@ en:
 
         If you can correct the problem, please try again.
 
-    email_reject_rate_limit_specified:
-      subject_template: "[%{site_name}] Email issue -- Rate limited"
-      text_body_template: |
-        We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work.
-
-        Reason: %{rate_limit_description}
-
     email_reject_invalid_post_action:
       subject_template: "[%{site_name}] Email issue -- Invalid Post Action"
       text_body_template: |
diff --git a/lib/email/processor.rb b/lib/email/processor.rb
index 3005b00d7..71c9b846f 100644
--- a/lib/email/processor.rb
+++ b/lib/email/processor.rb
@@ -2,18 +2,21 @@ module Email
 
   class Processor
 
-    def initialize(mail)
+    def initialize(mail, retry_on_rate_limit=true)
       @mail = mail
+      @retry_on_rate_limit = retry_on_rate_limit
     end
 
-    def self.process!(mail)
-      Email::Processor.new(mail).process!
+    def self.process!(mail, retry_on_rate_limit=true)
+      Email::Processor.new(mail, retry_on_rate_limit).process!
     end
 
     def process!
       begin
         receiver = Email::Receiver.new(@mail)
         receiver.process!
+      rescue RateLimiter::LimitExceeded
+        @retry_on_rate_limit ? Jobs.enqueue(:process_email, mail: @mail) : raise
       rescue Email::Receiver::BouncedEmailError => e
         # never reply to bounced emails
         log_email_process_failure(@mail, e)
@@ -49,7 +52,6 @@ module Email
         when ActiveRecord::Rollback                       then :email_reject_invalid_post
         when Email::Receiver::InvalidPostAction           then :email_reject_invalid_post_action
         when Discourse::InvalidAccess                     then :email_reject_invalid_access
-        when RateLimiter::LimitExceeded                   then :email_reject_rate_limit_specified
       end
 
       template_args = {}
@@ -61,10 +63,6 @@ module Email
         template_args[:post_error] = e.message
       end
 
-      if message_template == :email_reject_rate_limit_specified
-        template_args[:rate_limit_description] = e.description
-      end
-
       if message_template
         # inform the user about the rejection
         message = Mail::Message.new(mail_string)
diff --git a/spec/components/email/processor_spec.rb b/spec/components/email/processor_spec.rb
new file mode 100644
index 000000000..c30b09ad4
--- /dev/null
+++ b/spec/components/email/processor_spec.rb
@@ -0,0 +1,27 @@
+require "rails_helper"
+require "email/processor"
+
+describe Email::Processor do
+
+  describe "rate limits" do
+
+    let(:mail) { "From: foo@bar.com\nTo: bar@foo.com\nSubject: FOO BAR\n\nFoo foo bar bar?" }
+    let(:limit_exceeded) { RateLimiter::LimitExceeded.new(10) }
+
+    before do
+      Email::Receiver.any_instance.expects(:process!).raises(limit_exceeded)
+    end
+
+    it "enqueues a background job by default" do
+      Jobs.expects(:enqueue).with(:process_email, mail: mail)
+      Email::Processor.process!(mail)
+    end
+
+    it "doesn't enqueue a background job when retry is disabled" do
+      Jobs.expects(:enqueue).with(:process_email, mail: mail).never
+      expect { Email::Processor.process!(mail, false) }.to raise_error(limit_exceeded)
+    end
+
+  end
+
+end
diff --git a/spec/jobs/process_email_spec.rb b/spec/jobs/process_email_spec.rb
new file mode 100644
index 000000000..f740a8f2b
--- /dev/null
+++ b/spec/jobs/process_email_spec.rb
@@ -0,0 +1,12 @@
+require "rails_helper"
+
+describe Jobs::ProcessEmail do
+
+  let(:mail) { "From: foo@bar.com\nTo: bar@foo.com\nSubject: FOO BAR\n\nFoo foo bar bar?" }
+
+  it "process an email without retry" do
+    Email::Processor.expects(:process!).with(mail, false)
+    Jobs::ProcessEmail.new.execute(mail: mail)
+  end
+
+end