diff --git a/app/jobs/regular/group_smtp_email.rb b/app/jobs/regular/group_smtp_email.rb index 6970c673ff9..968bcf45406 100644 --- a/app/jobs/regular/group_smtp_email.rb +++ b/app/jobs/regular/group_smtp_email.rb @@ -83,24 +83,44 @@ module Jobs cc_addresses: cc_addresses, bcc_addresses: bcc_addresses, ) - Email::Sender.new(message, :group_smtp, recipient_user).send + + begin + Email::Sender.new(message, :group_smtp, recipient_user).send + rescue Net::ReadTimeout => err + # We can't be sure if the send actually failed or if ENTER . ENTER (to end + # the SMTP data sequence) just timed out, as is the case with Gmail occassionaly, + # where they can do this if they suspect you are sending spam. + Discourse.warn_exception( + err, + message: "Got SMTP read timeout when sending group SMTP email", + env: args, + ) + end # Create an incoming email record to avoid importing again from IMAP # server. While this may not be technically required if IMAP is not # currently enabled for the group, it will help a lot with the initial # sync if it is turned on at a later date. - IncomingEmail.create!( - user_id: post.user_id, - topic_id: post.topic_id, - post_id: post.id, - raw: message.to_s, - subject: message.subject, - message_id: message.message_id, - to_addresses: message.to, - cc_addresses: message.cc, - from_address: message.from, - created_via: IncomingEmail.created_via_types[:group_smtp], - ) + begin + IncomingEmail.create!( + user_id: post.user_id, + topic_id: post.topic_id, + post_id: post.id, + raw: message.to_s, + subject: message.subject, + message_id: message.message_id, + to_addresses: message.to, + cc_addresses: message.cc, + from_address: message.from, + created_via: IncomingEmail.created_via_types[:group_smtp], + ) + rescue => err + Discourse.warn_exception( + err, + message: "Failed to create IncomingEmail record when sending group SMTP email", + env: args, + ) + end end def quit_email_early? diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index f2287665849..7e4d723f881 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -96,7 +96,7 @@ smtp_force_tls = false smtp_open_timeout = 5 # Number of seconds to wait until timing-out a SMTP read(2) call -smtp_read_timeout = 5 +smtp_read_timeout = 30 # number of seconds to wait while attempting to open a SMTP connection only when # sending emails via group SMTP diff --git a/spec/jobs/regular/group_smtp_email_spec.rb b/spec/jobs/regular/group_smtp_email_spec.rb index cad54dd7578..b5ec592a74a 100644 --- a/spec/jobs/regular/group_smtp_email_spec.rb +++ b/spec/jobs/regular/group_smtp_email_spec.rb @@ -185,6 +185,18 @@ RSpec.describe Jobs::GroupSmtpEmail do expect(last_email.cc).to match_array(%w[otherguy@test.com cormac@lit.com]) end + it "does not retry the job if the IncomingEmail record is not created because of an error" do + Jobs.run_immediately! + IncomingEmail.expects(:create!).raises(StandardError) + expect { Jobs.enqueue(:group_smtp_email, **args) }.not_to raise_error + end + + it "does not retry the job on SMTP read timeouts, because we can't be sure if the send actually failed or if ENTER . ENTER just timed out" do + Jobs.run_immediately! + Email::Sender.any_instance.expects(:send).raises(Net::ReadTimeout) + expect { Jobs.enqueue(:group_smtp_email, **args) }.not_to raise_error + end + context "when there are cc_addresses" do it "has the cc_addresses and cc_user_ids filled in correctly" do job.execute(args)