From edc17dddb39d916769f110602b15c31d2947dd30 Mon Sep 17 00:00:00 2001 From: riking Date: Sun, 22 Jun 2014 15:04:24 -0700 Subject: [PATCH 1/6] Let's see if this works --- app/jobs/scheduled/poll_mailbox.rb | 4 ++++ app/mailers/rejection_mailer.rb | 8 ++++---- config/locales/server.en.yml | 4 ++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index e0f4c852a87..e1f16113976 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -28,6 +28,10 @@ module Jobs client_message = RejectionMailer.send_trust_level(message.from, message.body) Email::Sender.new(client_message, :email_reject_trust_level).send rescue Email::Receiver::ProcessingError => e + Rails.logger.error e + message = Mail::Message.new(mail_string) + client_message = RejectionMailer.send_rejection(message.from, message.body, e.message) + # inform admins about the error data = { limit_once_per: false, message_params: { source: mail, error: e }} GroupMessage.create(Group[:admins].name, :email_error_notification, data) diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index 754e51c214c..0bf9e2b6a98 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -3,11 +3,11 @@ require_dependency 'email/message_builder' class RejectionMailer < ActionMailer::Base include Email::BuildEmailHelper - def send_rejection(from, body) - build_email(from, template: 'email_error_notification', from: from, body: body) + def send_rejection(from, body, error) + build_email(from, template: 'email_error_notification', error: "#{error.message}\n\n#{error.backtrace.join("\n")}", source: body) end - def send_trust_level(from, body, to) - build_email(from, template: 'email_reject_trust_level', to: to) + def send_trust_level(from, body) + build_email(from, template: 'email_reject_trust_level') end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4795c82ba89..860627621c6 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1306,6 +1306,10 @@ en: `%{error}` + The original message follows. + + --- + %{source} email_reject_trust_level: From d2823fc5eeaaa7ded73714ae5f953268402bfdbf Mon Sep 17 00:00:00 2001 From: riking Date: Mon, 23 Jun 2014 17:16:56 -0700 Subject: [PATCH 2/6] More detailed email rejection responses --- app/jobs/scheduled/poll_mailbox.rb | 41 ++++++++++++++++++++---------- app/mailers/rejection_mailer.rb | 6 ++--- config/locales/server.en.yml | 38 ++++++++++++++++++++++++--- 3 files changed, 65 insertions(+), 20 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index e1f16113976..abaf263430b 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -22,23 +22,36 @@ module Jobs begin mail_string = mail.pop Email::Receiver.new(mail_string).process - rescue Email::Receiver::UserNotSufficientTrustLevelError + rescue => e # inform the user about the rejection message = Mail::Message.new(mail_string) - client_message = RejectionMailer.send_trust_level(message.from, message.body) - Email::Sender.new(client_message, :email_reject_trust_level).send - rescue Email::Receiver::ProcessingError => e - Rails.logger.error e - message = Mail::Message.new(mail_string) - client_message = RejectionMailer.send_rejection(message.from, message.body, e.message) + message_template = nil + case e + when Email::Receiver::UserNotSufficientTrustLevelError + message_template = :email_reject_trust_level + when Email::Receiver::UserNotFoundError + message_template = :email_reject_no_account + when Email::Receiver::EmptyEmailError + message_template = :email_reject_empty + when Email::Receiver::EmailUnparsableError + message_template = :email_reject_parsing + when ActiveRecord::Rollback + message_template = :email_reject_post_error + else + nil + end - # inform admins about the error - data = { limit_once_per: false, message_params: { source: mail, error: e }} - GroupMessage.create(Group[:admins].name, :email_error_notification, data) - rescue StandardError => e - # inform admins about the error - data = { limit_once_per: false, message_params: { source: mail, error: e }} - GroupMessage.create(Group[:admins].name, :email_error_notification, data) + if message_template + # Send message to the user + client_message = RejectionMailer.send_rejection(message.from, message.body, message_template.to_s, "#{e.message}\n\n#{e.backtrace.join("\n")}") + Email::Sender.new(client_message, message_template).send + else + Rails.logger.error e + + # If not known type, inform admins about the error + data = { limit_once_per: false, message_params: { from: message.from, source: message.body, error: "#{e.message}\n\n#{e.backtrace.join("\n")}" }} + GroupMessage.create(Group[:admins].name, :email_error_notification, data) + end ensure mail.delete end diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index 0bf9e2b6a98..324da3961d6 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -3,11 +3,11 @@ require_dependency 'email/message_builder' class RejectionMailer < ActionMailer::Base include Email::BuildEmailHelper - def send_rejection(from, body, error) - build_email(from, template: 'email_error_notification', error: "#{error.message}\n\n#{error.backtrace.join("\n")}", source: body) + def send_rejection(from, body, template, error) + build_email(from, template: template, error: error, source: body) end - def send_trust_level(from, body) + def send_trust_level(from, template) build_email(from, template: 'email_reject_trust_level') end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 860627621c6..c859cb3a407 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1302,9 +1302,13 @@ en: text_body_template: | This is an automated message. - Parsing an incoming email failed. Please review the following Error: + Parsing an incoming email from `%{from}` failed. Please review the following Error: - `%{error}` + --- + + %{error} + + --- The original message follows. @@ -1313,12 +1317,40 @@ en: %{source} email_reject_trust_level: - subject_template: "Message rejected" + subject_template: "Message rejected: Insufficient Trust Level" text_body_template: | The message you've send to %{to} was rejected by the system. You do not have the required trust to post new topics to this email address. + email_reject_no_account: + subject_template: "Message rejected: No Account" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + You do not have an account on the forum with this email address. + + email_reject_empty: + subject_template: "Message rejected: No Content" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + There was no recognized content in the email. + + email_reject_parsing: + subject_template: "Message rejected: Unparsable" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + The encoding was unknown or not supported. Try sending in UTF-8 plain text. + + email_reject_post_error: + subject_template: "Message rejected: Posting error" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + The Markdown could not be processed. + too_many_spam_flags: subject_template: "New account blocked" text_body_template: | From 8b5d2b835a5f2418eff7f7360ede2408a7e498ed Mon Sep 17 00:00:00 2001 From: riking Date: Mon, 23 Jun 2014 17:46:22 -0700 Subject: [PATCH 3/6] Add case for bad reply key --- app/jobs/scheduled/poll_mailbox.rb | 2 ++ config/locales/server.en.yml | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index abaf263430b..262ec0baa64 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -35,6 +35,8 @@ module Jobs message_template = :email_reject_empty when Email::Receiver::EmailUnparsableError message_template = :email_reject_parsing + when Email::Receiver::EmailLogNotFound + message_template = :email_reject_reply_key when ActiveRecord::Rollback message_template = :email_reject_post_error else diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c859cb3a407..a3ecb298ff4 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1351,6 +1351,13 @@ en: The Markdown could not be processed. + email_reject_reply_key: + subject_template: "Message rejected: Bad Reply Key" + text_body_template: | + The message you've send to %{to} was rejected by the system. + + The provided reply key is invalid. + too_many_spam_flags: subject_template: "New account blocked" text_body_template: | From 222db71dd7046349fc831e8e2ef8aeb0c05c22b7 Mon Sep 17 00:00:00 2001 From: riking Date: Mon, 23 Jun 2014 17:48:51 -0700 Subject: [PATCH 4/6] Cleanup --- app/jobs/scheduled/poll_mailbox.rb | 3 ++- config/locales/server.en.yml | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 262ec0baa64..d2162434d5d 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -40,7 +40,7 @@ module Jobs when ActiveRecord::Rollback message_template = :email_reject_post_error else - nil + message_template = nil end if message_template @@ -51,6 +51,7 @@ module Jobs Rails.logger.error e # If not known type, inform admins about the error + # (Add to above case with a good error message!) data = { limit_once_per: false, message_params: { from: message.from, source: message.body, error: "#{e.message}\n\n#{e.backtrace.join("\n")}" }} GroupMessage.create(Group[:admins].name, :email_error_notification, data) end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index a3ecb298ff4..a14b0d0abe2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1351,6 +1351,8 @@ en: The Markdown could not be processed. + %{error} + email_reject_reply_key: subject_template: "Message rejected: Bad Reply Key" text_body_template: | From 420d3d651b268489291fb1570fcd5e6bb84a1580 Mon Sep 17 00:00:00 2001 From: riking Date: Mon, 23 Jun 2014 17:51:22 -0700 Subject: [PATCH 5/6] Make sure from is present --- app/mailers/rejection_mailer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/mailers/rejection_mailer.rb b/app/mailers/rejection_mailer.rb index 324da3961d6..1d37395be9f 100644 --- a/app/mailers/rejection_mailer.rb +++ b/app/mailers/rejection_mailer.rb @@ -4,7 +4,7 @@ class RejectionMailer < ActionMailer::Base include Email::BuildEmailHelper def send_rejection(from, body, template, error) - build_email(from, template: template, error: error, source: body) + build_email(from, from: from, template: template, error: error, source: body) end def send_trust_level(from, template) From 7ab5d3c018a49223cd632ab67d5de639ff94270e Mon Sep 17 00:00:00 2001 From: riking Date: Mon, 23 Jun 2014 18:11:50 -0700 Subject: [PATCH 6/6] Fix specs --- spec/jobs/poll_mailbox_spec.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb index 71d5e4b687b..a189f155b2e 100644 --- a/spec/jobs/poll_mailbox_spec.rb +++ b/spec/jobs/poll_mailbox_spec.rb @@ -75,7 +75,7 @@ describe Jobs::PollMailbox do client_message = mock sender = mock - RejectionMailer.expects(:send_trust_level).returns(client_message) + RejectionMailer.expects(:send_rejection).returns(client_message) Email::Sender.expects(:new).with(client_message, :email_reject_trust_level).returns(sender) sender.expects(:send) @@ -90,6 +90,8 @@ describe Jobs::PollMailbox do Email::Receiver::EmptyEmailError, Email::Receiver::UserNotFoundError, Email::Receiver::EmailLogNotFound, + ActiveRecord::Rollback, + TypeError ].each do |exception| it "deletes email on #{exception}" do @@ -103,10 +105,16 @@ describe Jobs::PollMailbox do it "informs admins on any other error" do error = TypeError.new - data = { limit_once_per: false, message_params: { source: email, error: error }} - + error.set_backtrace(["Hi", "Bye"]) receiver.expects(:process).raises(error) email.expects(:delete) + + Mail::Message.expects(:new).with(email_string).returns(email) + email.expects(:from).twice + email.expects(:body).twice + + data = { limit_once_per: false, message_params: { from: email.from, source: email.body, error: "#{error.message}\n\n#{error.backtrace.join("\n")}" }} + GroupMessage.expects(:create).with("admins", :email_error_notification, data) poller.handle_mail(email)