mirror of
https://github.com/discourse/discourse.git
synced 2025-02-24 16:49:57 +08:00
DEV: Have group smtp email job retry (#31202)
Likely we want the group smtp email job to retry. Also added a check to see if we already have an email log entry for the message to avoid possible duplicates on retry. Related previous commit: ed47b550266e1ab669c756b0ecb48d1685b08fee
This commit is contained in:
parent
d4e3595a07
commit
bc29fbeac8
@ -84,6 +84,9 @@ module Jobs
|
|||||||
bcc_addresses: bcc_addresses,
|
bcc_addresses: bcc_addresses,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Idempotency check – if the EmailLog already exists, do not send again.
|
||||||
|
return if EmailLog.exists?(message_id: message.message_id)
|
||||||
|
|
||||||
begin
|
begin
|
||||||
Email::Sender.new(message, :group_smtp, recipient_user).send
|
Email::Sender.new(message, :group_smtp, recipient_user).send
|
||||||
rescue Net::ReadTimeout => err
|
rescue Net::ReadTimeout => err
|
||||||
@ -95,6 +98,7 @@ module Jobs
|
|||||||
message: "Got SMTP read timeout when sending group SMTP email",
|
message: "Got SMTP read timeout when sending group SMTP email",
|
||||||
env: args,
|
env: args,
|
||||||
)
|
)
|
||||||
|
raise err # Re-raise the error so Sidekiq's retry mechanism kicks in.
|
||||||
end
|
end
|
||||||
|
|
||||||
# Create an incoming email record to avoid importing again from IMAP
|
# Create an incoming email record to avoid importing again from IMAP
|
||||||
|
@ -191,12 +191,6 @@ RSpec.describe Jobs::GroupSmtpEmail do
|
|||||||
expect { Jobs.enqueue(:group_smtp_email, **args) }.not_to raise_error
|
expect { Jobs.enqueue(:group_smtp_email, **args) }.not_to raise_error
|
||||||
end
|
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
|
context "when there are cc_addresses" do
|
||||||
it "has the cc_addresses and cc_user_ids filled in correctly" do
|
it "has the cc_addresses and cc_user_ids filled in correctly" do
|
||||||
job.execute(args)
|
job.execute(args)
|
||||||
@ -322,4 +316,38 @@ RSpec.describe Jobs::GroupSmtpEmail do
|
|||||||
).to eq(true)
|
).to eq(true)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "re-raises Net::ReadTimeout to trigger Sidekiq retries" do
|
||||||
|
allow_any_instance_of(Email::Sender).to receive(:send).and_raise(
|
||||||
|
Net::ReadTimeout.new("timeout"),
|
||||||
|
)
|
||||||
|
expect { job.execute(args) }.to raise_error(Net::ReadTimeout)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "does not send email if an EmailLog with the same message_id already exists" do
|
||||||
|
message = Mail::Message.new(body: "hello", to: "myemail@example.invalid")
|
||||||
|
message.message_id = "unique_message_id"
|
||||||
|
GroupSmtpMailer
|
||||||
|
.expects(:send_mail)
|
||||||
|
.with(
|
||||||
|
group,
|
||||||
|
"test@test.com",
|
||||||
|
post,
|
||||||
|
cc_addresses: %w[otherguy@test.com cormac@lit.com],
|
||||||
|
bcc_addresses: [],
|
||||||
|
)
|
||||||
|
.returns(message)
|
||||||
|
EmailLog.expects(:exists?).with(message_id: message.message_id).returns(true)
|
||||||
|
Email::Sender.any_instance.expects(:send).never
|
||||||
|
|
||||||
|
job.execute(args)
|
||||||
|
expect(ActionMailer::Base.deliveries.count).to eq(0)
|
||||||
|
end
|
||||||
|
|
||||||
|
it "retries the job on SMTP read timeouts when there are no duplicates" do
|
||||||
|
Jobs.run_immediately!
|
||||||
|
EmailLog.stubs(:exists?).with(message_id: anything).returns(false)
|
||||||
|
Email::Sender.any_instance.expects(:send).raises(Net::ReadTimeout)
|
||||||
|
expect { Jobs.enqueue(:group_smtp_email, **args) }.to raise_error(Net::ReadTimeout)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
Loading…
x
Reference in New Issue
Block a user