From 49af26279dd85f1ca49195ff7cf644c4d3adb04b Mon Sep 17 00:00:00 2001 From: "Fabien \"egg\" O'Carroll" Date: Wed, 11 Jan 2023 09:30:15 +0700 Subject: [PATCH] Updated MailgunEmailSuppressionList to only handle previous bounces We're seeing behaviour from Mailgun where permanent failures with a 5xx error code are not being added to their internal suppression list, which is resulting in the Ghost list becoming out of sync with Mailgun. Rather than adding emails to the suppression list when Mailgun does, we're instead going to add emails _after_ Mailgun does, by waiting for an error code which tells us the email is already on the suppression list. Those codes are 605 for previous bounces and 607 for previous spam complaints. --- .../MailgunEmailSuppressionList.js | 2 +- .../mailgun-email-suppression-list.test.js | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/ghost/core/core/server/services/email-suppression-list/MailgunEmailSuppressionList.js b/ghost/core/core/server/services/email-suppression-list/MailgunEmailSuppressionList.js index 26a863be99..3370573d07 100644 --- a/ghost/core/core/server/services/email-suppression-list/MailgunEmailSuppressionList.js +++ b/ghost/core/core/server/services/email-suppression-list/MailgunEmailSuppressionList.js @@ -101,7 +101,7 @@ class MailgunEmailSuppressionList extends AbstractEmailSuppressionList { if (!Number.isInteger(event.error?.code)) { return; } - if (event.error.code < 500 || event.error.code > 599 && event.error.code !== 605) { + if (event.error.code !== 607 && event.error.code !== 605) { return; } } diff --git a/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js b/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js index 38690c284c..e8a1167930 100644 --- a/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js +++ b/ghost/core/test/integration/services/mailgun-email-suppression-list.test.js @@ -58,7 +58,7 @@ describe('MailgunEmailSuppressionList', function () { assert.equal(memberAfter.email_suppression.info.reason, 'fail'); }); - it('Can handle permanent failure events with an error code of 6xx', async function () { + it('Can handle permanent failure events with an error code of 607', async function () { const emailBatch = fixtureManager.get('email_batches', 0); const emailRecipient = fixtureManager.get('email_recipients', 1); @@ -71,10 +71,8 @@ describe('MailgunEmailSuppressionList', function () { const {body: {members: [member]}} = await agent.get(`/members/${memberId}`); assert.equal(member.email_suppression.suppressed, false, 'This test requires a member that does not have a suppressed email'); - const errorCode = 600 + Math.floor(Math.random() * 100); - events = [createPermanentFailedEvent({ - errorCode: errorCode === 605 ? 606 : errorCode, // Random number between 600-699, but not 605 + errorCode: 607, providerId, timestamp, recipient @@ -87,8 +85,8 @@ describe('MailgunEmailSuppressionList', function () { await DomainEvents.allSettled(); const {body: {members: [memberAfter]}} = await agent.get(`/members/${memberId}`); - assert.equal(memberAfter.email_suppression.suppressed, false, 'The member should not have a suppressed email'); - assert.equal(memberAfter.email_suppression.info, null); + assert.equal(memberAfter.email_suppression.suppressed, true, 'The member should have a suppressed email'); + assert.equal(memberAfter.email_suppression.info.reason, 'fail'); }); it('Can handle permanent failure events with an error code of 4xx', async function () { @@ -149,8 +147,8 @@ describe('MailgunEmailSuppressionList', function () { await DomainEvents.allSettled(); const {body: {members: [memberAfter]}} = await agent.get(`/members/${memberId}`); - assert.equal(memberAfter.email_suppression.suppressed, true, 'The member should have a suppressed email'); - assert.equal(memberAfter.email_suppression.info.reason, 'fail'); + assert.equal(memberAfter.email_suppression.suppressed, false, 'The member should not have a suppressed email'); + assert.equal(memberAfter.email_suppression.info, null); }); it('Can handle complaint events', async function () {