From 1a7f268c9ab58e037b968f848556221b5d312a99 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 6 May 2026 15:29:36 +0100 Subject: [PATCH 1/4] Add column to store bounce details to deliveries We want to send a daily notification of any bounced submission emails to the admins for a form. In the notification email, we want to include different content depending on whether we can identify the bounce as a hard or soft bounce, with the scope to be more specific in the future for different bounce types. Add a `failure_details` jsonb column to the deliveries table to store additional failure details when a delivery fails. --- .../20260506142828_add_failure_details_to_deliveries.rb | 5 +++++ db/schema.rb | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20260506142828_add_failure_details_to_deliveries.rb diff --git a/db/migrate/20260506142828_add_failure_details_to_deliveries.rb b/db/migrate/20260506142828_add_failure_details_to_deliveries.rb new file mode 100644 index 000000000..ad74bb770 --- /dev/null +++ b/db/migrate/20260506142828_add_failure_details_to_deliveries.rb @@ -0,0 +1,5 @@ +class AddFailureDetailsToDeliveries < ActiveRecord::Migration[8.1] + def change + add_column :deliveries, :failure_details, :jsonb + end +end diff --git a/db/schema.rb b/db/schema.rb index 74b36471c..ce96ed055 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_03_17_125725) do +ActiveRecord::Schema[8.1].define(version: 2026_05_06_142828) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -21,6 +21,7 @@ t.string "delivery_reference" t.string "delivery_schedule", default: "immediate", null: false, comment: "Either 'immediate' if the delivery is for a single submission or a value representing the schedule for sending multiple submissions." t.datetime "failed_at" + t.jsonb "failure_details" t.string "failure_reason" t.datetime "last_attempt_at" t.datetime "updated_at", null: false From 890c7e9abfa8678b72f2ef102e66a5dd754d8d5b Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 6 May 2026 15:47:21 +0100 Subject: [PATCH 2/4] Store the bounce details on the delivery When handling a bounce, store the JSON blob continaing detailed information about the bounce on the delivery. This will allows us to inspect this information when sending daily notification emails about bounces to form admins. Update the specs to remove the additional "when there is a bounce object with detailed information" contexts and instead include the assertions that we log the bounce details in the existing contexts. --- ...e_submission_bounces_and_complaints_job.rb | 1 + ...mission_bounces_and_complaints_job_spec.rb | 318 ++++++++---------- 2 files changed, 142 insertions(+), 177 deletions(-) diff --git a/app/jobs/receive_submission_bounces_and_complaints_job.rb b/app/jobs/receive_submission_bounces_and_complaints_job.rb index be0b917e4..7835b7858 100644 --- a/app/jobs/receive_submission_bounces_and_complaints_job.rb +++ b/app/jobs/receive_submission_bounces_and_complaints_job.rb @@ -46,6 +46,7 @@ def process_bounce(delivery, submission, ses_message) delivery.update!( failed_at: bounced_timestamp, failure_reason: "bounced", + failure_details: bounce_object, ) end diff --git a/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb b/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb index e0cd8e0a9..dad2efac6 100644 --- a/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb +++ b/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb @@ -9,10 +9,28 @@ let(:sqs_message_id) { "sqs-message-id" } let(:sqs_message) { instance_double(Aws::SQS::Types::Message, message_id: sqs_message_id, receipt_handle:, body: sns_message_body) } let(:messages) { [] } + let(:sns_message_timestamp) { "2025-05-09T10:25:43.972Z" } let(:sns_message_body) { { "Message" => ses_message_body.to_json, "Timestamp" => sns_message_timestamp }.to_json } let(:event_type) { "Bounce" } - let(:ses_message_body) { { "mail" => { "messageId" => delivery_reference }, "eventType": event_type } } + let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } + let(:ses_message_body) do + { + "mail" => { "messageId" => delivery_reference }, + "eventType" => event_type, + "bounce" => bounce, + } + end + let(:bounce) do + { + "bounceType" => "Permanent", + "bounceSubType" => "General", + "bouncedRecipients" => bounced_recipients, + "timestamp" => bounce_timestamp, + } + end + let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } + let(:delivery_reference) { "delivery-reference" } let(:reference) { "submission-reference" } let(:mode) { "form" } @@ -50,20 +68,12 @@ end context "when it is for a live submission" do - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:ses_message_body) do - { - "mail" => { "messageId" => delivery_reference }, - "eventType" => event_type, - "bounce" => { "timestamp" => bounce_timestamp }, - } - end - - it "updates the delivery record's failed_at and failure_reason" do + it "updates the delivery record with the bounce details" do perform_enqueued_jobs expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) expect(delivery.reload.failure_reason).to eq("bounced") + expect(delivery.reload.failure_details).to eq(bounce) end it "logs form event with correct details" do @@ -82,13 +92,44 @@ "sns_message_timestamp" => sns_message_timestamp, "job_id" => @job_id, "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", + "ses_bounce" => hash_including( + "bounce_type" => "Permanent", + "bounce_sub_type" => "General", + "bounced_recipients" => [ + hash_including( + "email_address" => "bounce@example.com", + ), + ], + ), )) end it "alerts to Sentry that there was a bounced delivery" do allow(Sentry).to receive(:capture_message) perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message) + expect(Sentry).to have_received(:capture_message).with( + a_string_including("Submission email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), + fingerprint: ["{{ default }}", submission.form_id], + extra: hash_including( + ses_bounce: hash_including( + bounce_type: "Permanent", + bounce_sub_type: "General", + ), + ), + ) + end + + it "does not include bounced recipients in the Sentry event" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).not_to have_received(:capture_message).with( + anything, + extra: hash_including( + ses_bounce: hash_including( + :bounced_recipients, + ), + ), + ) end end @@ -117,59 +158,6 @@ expect(Sentry).not_to have_received(:capture_message) end end - - context "when there is a bounce object with detailed information" do - let(:ses_message_body) { { "mail" => { "messageId" => delivery_reference }, "eventType" => event_type, "bounce" => bounce } } - let(:bounce) { { "bounceType" => "Permanent", "bounceSubType" => "General", "bouncedRecipients" => bounced_recipients, "timestamp" => bounce_timestamp } } - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - - it "logs the bounce details" do - perform_enqueued_jobs - - expect(log_lines).to include( - hash_including( - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - ), - ) - end - - it "includes bounce details in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Submission email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end - end end describe "processing complaint notifications" do @@ -231,15 +219,25 @@ { "mail" => { "messageId" => delivery_reference }, "eventType" => event_type, - "bounce" => { "timestamp" => bounce_timestamp }, + "bounce" => bounce, + } + end + let(:bounce) do + { + "bounceType" => "Permanent", + "bounceSubType" => "General", + "bouncedRecipients" => bounced_recipients, + "timestamp" => bounce_timestamp, } end + let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - it "updates the delivery record's failed_at and failure_reason" do + it "updates the delivery record with bounce details" do perform_enqueued_jobs expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) expect(delivery.reload.failure_reason).to eq("bounced") + expect(delivery.reload.failure_details).to eq(bounce) end it "logs form event with correct details" do @@ -259,13 +257,44 @@ "sns_message_timestamp" => sns_message_timestamp, "job_id" => @job_id, "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", + "ses_bounce" => hash_including( + "bounce_type" => "Permanent", + "bounce_sub_type" => "General", + "bounced_recipients" => [ + hash_including( + "email_address" => "bounce@example.com", + ), + ], + ), )) end it "alerts to Sentry that there was a bounced delivery" do allow(Sentry).to receive(:capture_message) perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message) + expect(Sentry).to have_received(:capture_message).with( + a_string_including("Daily submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), + fingerprint: ["{{ default }}", submission.form_id], + extra: hash_including( + ses_bounce: hash_including( + bounce_type: "Permanent", + bounce_sub_type: "General", + ), + ), + ) + end + + it "does not include bounced recipients in the Sentry event" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).not_to have_received(:capture_message).with( + anything, + extra: hash_including( + ses_bounce: hash_including( + :bounced_recipients, + ), + ), + ) end end @@ -294,59 +323,6 @@ expect(Sentry).not_to have_received(:capture_message) end end - - context "when there is a bounce object with detailed information" do - let(:ses_message_body) { { "mail" => { "messageId" => delivery_reference }, "eventType" => event_type, "bounce" => bounce } } - let(:bounce) { { "bounceType" => "Permanent", "bounceSubType" => "General", "bouncedRecipients" => bounced_recipients, "timestamp" => bounce_timestamp } } - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - - it "logs the bounce details" do - perform_enqueued_jobs - - expect(log_lines).to include( - hash_including( - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - ), - ) - end - - it "includes bounce details in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Daily submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end - end end describe "processing complaint notifications" do @@ -391,15 +367,25 @@ { "mail" => { "messageId" => delivery_reference }, "eventType" => event_type, - "bounce" => { "timestamp" => bounce_timestamp }, + "bounce" => bounce, + } + end + let(:bounce) do + { + "bounceType" => "Permanent", + "bounceSubType" => "General", + "bouncedRecipients" => bounced_recipients, + "timestamp" => bounce_timestamp, } end + let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - it "updates the delivery record's failed_at and failure_reason" do + it "updates the delivery record with bounce details" do perform_enqueued_jobs expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) expect(delivery.reload.failure_reason).to eq("bounced") + expect(delivery.reload.failure_details).to eq(bounce) end it "logs form event with correct details" do @@ -419,13 +405,44 @@ "sns_message_timestamp" => sns_message_timestamp, "job_id" => @job_id, "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", + "ses_bounce" => hash_including( + "bounce_type" => "Permanent", + "bounce_sub_type" => "General", + "bounced_recipients" => [ + hash_including( + "email_address" => "bounce@example.com", + ), + ], + ), )) end it "alerts to Sentry that there was a bounced delivery" do allow(Sentry).to receive(:capture_message) perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message) + expect(Sentry).to have_received(:capture_message).with( + a_string_including("Weekly submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), + fingerprint: ["{{ default }}", submission.form_id], + extra: hash_including( + ses_bounce: hash_including( + bounce_type: "Permanent", + bounce_sub_type: "General", + ), + ), + ) + end + + it "does not include bounced recipients in the Sentry event" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).not_to have_received(:capture_message).with( + anything, + extra: hash_including( + ses_bounce: hash_including( + :bounced_recipients, + ), + ), + ) end end @@ -454,59 +471,6 @@ expect(Sentry).not_to have_received(:capture_message) end end - - context "when there is a bounce object with detailed information" do - let(:ses_message_body) { { "mail" => { "messageId" => delivery_reference }, "eventType" => event_type, "bounce" => bounce } } - let(:bounce) { { "bounceType" => "Permanent", "bounceSubType" => "General", "bouncedRecipients" => bounced_recipients, "timestamp" => bounce_timestamp } } - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - - it "logs the bounce details" do - perform_enqueued_jobs - - expect(log_lines).to include( - hash_including( - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - ), - ) - end - - it "includes bounce details in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Weekly submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end - end end describe "processing complaint notifications" do From 76eb108fc816014ddd24ddc8f7c81a60c3f386d6 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 6 May 2026 16:26:38 +0100 Subject: [PATCH 3/4] Move duplicated examples to shared examples --- ...mission_bounces_and_complaints_job_spec.rb | 406 +++++------------- 1 file changed, 111 insertions(+), 295 deletions(-) diff --git a/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb b/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb index dad2efac6..d57fcfe04 100644 --- a/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb +++ b/spec/jobs/receive_submission_bounces_and_complaints_job_spec.rb @@ -31,6 +31,8 @@ end let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } + let(:extra_log_fields) { {} } + let(:delivery_reference) { "delivery-reference" } let(:reference) { "submission-reference" } let(:mode) { "form" } @@ -49,6 +51,98 @@ allow(CloudWatchService).to receive(:record_job_started_metric) end + shared_examples "recording bounce details on delivery" do + it "updates the delivery record with the bounce details" do + perform_enqueued_jobs + + expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) + expect(delivery.reload.failure_reason).to eq("bounced") + expect(delivery.reload.failure_details).to eq(bounce) + end + end + + shared_examples "alerting Sentry about a bounced delivery" do |message_prefix| + it "alerts to Sentry that there was a bounced delivery" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).to have_received(:capture_message).with( + a_string_including("#{message_prefix} for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), + fingerprint: ["{{ default }}", submission.form_id], + extra: hash_including( + ses_bounce: hash_including( + bounce_type: "Permanent", + bounce_sub_type: "General", + ), + ), + ) + end + + it "does not include bounced recipients in the Sentry event" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).not_to have_received(:capture_message).with( + anything, + extra: hash_including( + ses_bounce: hash_including(:bounced_recipients), + ), + ) + end + end + + shared_examples "logging a bounce event" do |event_name| + it "logs form event with correct details" do + perform_enqueued_jobs + + expect(log_lines).to include(hash_including( + "level" => "INFO", + "message" => "Form event", + "event" => event_name, + "form_id" => submission.form_id, + "preview" => "false", + "delivery_reference" => delivery_reference, + "delivery_id" => delivery.id, + "sqs_message_id" => sqs_message_id, + "sns_message_timestamp" => sns_message_timestamp, + "job_id" => @job_id, + "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", + "ses_bounce" => hash_including( + "bounce_type" => "Permanent", + "bounce_sub_type" => "General", + "bounced_recipients" => [ + hash_including("email_address" => "bounce@example.com"), + ], + ), + **extra_log_fields, + )) + end + end + + shared_examples "preview submission bounce behaviour" do |event_name| + let(:mode) { "preview-live" } + + it "does not set failed_at on the delivery" do + perform_enqueued_jobs + expect(delivery.reload.failed_at).to be_nil + end + + it "logs form event with preview flag" do + perform_enqueued_jobs + + expect(log_lines).to include(hash_including( + "level" => "INFO", + "message" => "Form event", + "event" => event_name, + "preview" => "true", + )) + end + + it "does not alert to Sentry" do + allow(Sentry).to receive(:capture_message) + perform_enqueued_jobs + expect(Sentry).not_to have_received(:capture_message) + end + end + describe "CloudWatch metrics" do let(:messages) { [sqs_message] } @@ -68,95 +162,15 @@ end context "when it is for a live submission" do - it "updates the delivery record with the bounce details" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - expect(delivery.reload.failure_details).to eq(bounce) - end - - it "logs form event with correct details" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_submission_bounced", - "form_id" => submission.form_id, - "submission_reference" => reference, - "preview" => "false", - "delivery_reference" => delivery_reference, - "delivery_id" => delivery.id, - "sqs_message_id" => sqs_message_id, - "sns_message_timestamp" => sns_message_timestamp, - "job_id" => @job_id, - "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - )) - end + let(:extra_log_fields) { { "submission_reference" => reference } } - it "alerts to Sentry that there was a bounced delivery" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Submission email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end + it_behaves_like "recording bounce details on delivery" + it_behaves_like "logging a bounce event", "form_submission_bounced" + it_behaves_like "alerting Sentry about a bounced delivery", "Submission email bounced" end context "when it is for a preview submission" do - let(:mode) { "preview-live" } - - it "does not set failed_at on the delivery" do - perform_enqueued_jobs - expect(delivery.reload.failed_at).to be_nil - end - - it "logs form event with preview flag" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_submission_bounced", - "preview" => "true", - )) - end - - it "does not alert to Sentry" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message) - end + it_behaves_like "preview submission bounce behaviour", "form_submission_bounced" end end @@ -214,114 +228,15 @@ end context "when it is for a live submission" do - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:ses_message_body) do - { - "mail" => { "messageId" => delivery_reference }, - "eventType" => event_type, - "bounce" => bounce, - } - end - let(:bounce) do - { - "bounceType" => "Permanent", - "bounceSubType" => "General", - "bouncedRecipients" => bounced_recipients, - "timestamp" => bounce_timestamp, - } - end - let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - - it "updates the delivery record with bounce details" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - expect(delivery.reload.failure_details).to eq(bounce) - end - - it "logs form event with correct details" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_daily_batch_email_bounced", - "form_id" => submission.form_id, - "preview" => "false", - "delivery_reference" => delivery_reference, - "delivery_id" => delivery.id, - "delivery_schedule" => "daily", - "batch_begin_at" => delivery.batch_begin_at, - "sqs_message_id" => sqs_message_id, - "sns_message_timestamp" => sns_message_timestamp, - "job_id" => @job_id, - "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - )) - end - - it "alerts to Sentry that there was a bounced delivery" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Daily submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end + let(:extra_log_fields) { { "delivery_schedule" => "daily", "batch_begin_at" => delivery.batch_begin_at } } + + it_behaves_like "recording bounce details on delivery" + it_behaves_like "logging a bounce event", "form_daily_batch_email_bounced" + it_behaves_like "alerting Sentry about a bounced delivery", "Daily submission batch email bounced" end context "when it is for a preview submission" do - let(:mode) { "preview-live" } - - it "does not set failed_at on the delivery" do - perform_enqueued_jobs - expect(delivery.reload.failed_at).to be_nil - end - - it "logs form event with preview flag" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_daily_batch_email_bounced", - "preview" => "true", - )) - end - - it "does not alert to Sentry" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message) - end + it_behaves_like "preview submission bounce behaviour", "form_daily_batch_email_bounced" end end @@ -362,114 +277,15 @@ end context "when it is for a live submission" do - let(:bounce_timestamp) { "2023-01-01T12:00:00Z" } - let(:ses_message_body) do - { - "mail" => { "messageId" => delivery_reference }, - "eventType" => event_type, - "bounce" => bounce, - } - end - let(:bounce) do - { - "bounceType" => "Permanent", - "bounceSubType" => "General", - "bouncedRecipients" => bounced_recipients, - "timestamp" => bounce_timestamp, - } - end - let(:bounced_recipients) { [{ "emailAddress" => "bounce@example.com" }] } - - it "updates the delivery record with bounce details" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - expect(delivery.reload.failure_details).to eq(bounce) - end - - it "logs form event with correct details" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_weekly_batch_email_bounced", - "form_id" => submission.form_id, - "preview" => "false", - "delivery_reference" => delivery_reference, - "delivery_id" => delivery.id, - "delivery_schedule" => "weekly", - "batch_begin_at" => delivery.batch_begin_at, - "sqs_message_id" => sqs_message_id, - "sns_message_timestamp" => sns_message_timestamp, - "job_id" => @job_id, - "job_class" => "ReceiveSubmissionBouncesAndComplaintsJob", - "ses_bounce" => hash_including( - "bounce_type" => "Permanent", - "bounce_sub_type" => "General", - "bounced_recipients" => [ - hash_including( - "email_address" => "bounce@example.com", - ), - ], - ), - )) - end - - it "alerts to Sentry that there was a bounced delivery" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).to have_received(:capture_message).with( - a_string_including("Weekly submission batch email bounced for form #{submission.form_id} - ReceiveSubmissionBouncesAndComplaintsJob"), - fingerprint: ["{{ default }}", submission.form_id], - extra: hash_including( - ses_bounce: hash_including( - bounce_type: "Permanent", - bounce_sub_type: "General", - ), - ), - ) - end - - it "does not include bounced recipients in the Sentry event" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message).with( - anything, - extra: hash_including( - ses_bounce: hash_including( - :bounced_recipients, - ), - ), - ) - end + let(:extra_log_fields) { { "delivery_schedule" => "weekly", "batch_begin_at" => delivery.batch_begin_at } } + + it_behaves_like "recording bounce details on delivery" + it_behaves_like "logging a bounce event", "form_weekly_batch_email_bounced" + it_behaves_like "alerting Sentry about a bounced delivery", "Weekly submission batch email bounced" end context "when it is for a preview submission" do - let(:mode) { "preview-live" } - - it "does not set failed_at on the delivery" do - perform_enqueued_jobs - expect(delivery.reload.failed_at).to be_nil - end - - it "logs form event with preview flag" do - perform_enqueued_jobs - - expect(log_lines).to include(hash_including( - "level" => "INFO", - "message" => "Form event", - "event" => "form_weekly_batch_email_bounced", - "preview" => "true", - )) - end - - it "does not alert to Sentry" do - allow(Sentry).to receive(:capture_message) - perform_enqueued_jobs - expect(Sentry).not_to have_received(:capture_message) - end + it_behaves_like "preview submission bounce behaviour", "form_weekly_batch_email_bounced" end end From 40f6f14422fe8f08aa4525a51934ed0718c84b3b Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Wed, 6 May 2026 16:34:46 +0100 Subject: [PATCH 4/4] Clear failure_details when delivery is retried --- app/models/delivery.rb | 1 + spec/models/delivery_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/delivery.rb b/app/models/delivery.rb index 806c80bb6..7e5390e6d 100644 --- a/app/models/delivery.rb +++ b/app/models/delivery.rb @@ -32,6 +32,7 @@ def new_attempt! delivered_at: nil, failed_at: nil, failure_reason: nil, + failure_details: nil, ) end end diff --git a/spec/models/delivery_spec.rb b/spec/models/delivery_spec.rb index f85506fb8..0eb25728d 100644 --- a/spec/models/delivery_spec.rb +++ b/spec/models/delivery_spec.rb @@ -94,10 +94,11 @@ delivered_at: 3.hours.ago, failed_at: 2.hours.ago, failure_reason: "error", + failure_details: { "foo" => "bar" }, ) end - it "updates last_attempt_at and clears delivered_at, failed_at and failure_reason" do + it "updates last_attempt_at and clears delivered_at and failure attributes" do delivery.new_attempt! delivery.reload @@ -105,6 +106,7 @@ expect(delivery.delivered_at).to be_nil expect(delivery.failed_at).to be_nil expect(delivery.failure_reason).to be_nil + expect(delivery.failure_details).to be_nil end end end