DEV: Increase default SMTP read timeout to 30s (#25763)

A while ago we increased group SMTP read and open timeouts
to address issues we were seeing with Gmail sometimes giving
really long timeouts for these values. The commit was:

3e639e4aa7

Now, we want to increase all SMTP read timeouts to 30s,
since the 5s is too low sometimes, and the ruby Net::SMTP
stdlib also defaults to 30s.

Also, we want to slightly tweak the group smtp email job
not to fail if the IncomingEmail log fails to create, or if
a ReadTimeout is encountered, to avoid retrying the job in sidekiq
again and sending the same email out.

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?

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

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)