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/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/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 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..d57fcfe04 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,30 @@ 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(:extra_log_fields) { {} } + let(:delivery_reference) { "delivery-reference" } let(:reference) { "submission-reference" } let(:mode) { "form" } @@ -31,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] } @@ -50,125 +162,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" => { "timestamp" => bounce_timestamp }, - } - end + let(:extra_log_fields) { { "submission_reference" => reference } } - it "updates the delivery record's failed_at and failure_reason" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - 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", - )) - 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) - 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 - 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 + it_behaves_like "preview submission bounce behaviour", "form_submission_bounced" end end @@ -226,126 +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" => { "timestamp" => bounce_timestamp }, - } - end - - it "updates the delivery record's failed_at and failure_reason" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - 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", - )) - 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) - end - end + let(:extra_log_fields) { { "delivery_schedule" => "daily", "batch_begin_at" => delivery.batch_begin_at } } - 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 "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 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 + context "when it is for a preview submission" do + it_behaves_like "preview submission bounce behaviour", "form_daily_batch_email_bounced" end end @@ -386,126 +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" => { "timestamp" => bounce_timestamp }, - } - end - - it "updates the delivery record's failed_at and failure_reason" do - perform_enqueued_jobs - - expect(delivery.reload.failed_at).to eq(Time.zone.parse(bounce_timestamp)) - expect(delivery.reload.failure_reason).to eq("bounced") - 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", - )) - 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) - end - end + let(:extra_log_fields) { { "delivery_schedule" => "weekly", "batch_begin_at" => delivery.batch_begin_at } } - 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 "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 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 + context "when it is for a preview submission" do + it_behaves_like "preview submission bounce behaviour", "form_weekly_batch_email_bounced" 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