diff --git a/ghost/email-events/lib/EmailBouncedEvent.js b/ghost/email-events/lib/EmailBouncedEvent.js index fe70f890ff..c6f2e179da 100644 --- a/ghost/email-events/lib/EmailBouncedEvent.js +++ b/ghost/email-events/lib/EmailBouncedEvent.js @@ -4,7 +4,7 @@ module.exports = class EmailBouncedEvent { * @type {string} */ id; - + /** * @readonly * @type {string} @@ -25,7 +25,7 @@ module.exports = class EmailBouncedEvent { /** * @readonly - * @type {{message: string, code: number, enhancedCode: string | null}} + * @type {{message: string, code: number, enhancedCode: string | null}|null} */ error; diff --git a/ghost/email-events/lib/EmailTemporaryBouncedEvent.js b/ghost/email-events/lib/EmailTemporaryBouncedEvent.js index 6c277b0f35..f8159038a4 100644 --- a/ghost/email-events/lib/EmailTemporaryBouncedEvent.js +++ b/ghost/email-events/lib/EmailTemporaryBouncedEvent.js @@ -25,7 +25,7 @@ module.exports = class EmailTemporaryBouncedEvent { /** * @readonly - * @type {{message: string, code: number, enhancedCode: string | null}} + * @type {{message: string, code: number, enhancedCode: string | null}|null} */ error; diff --git a/ghost/email-service/lib/email-event-storage.js b/ghost/email-service/lib/email-event-storage.js index 6a862604c6..978182f948 100644 --- a/ghost/email-service/lib/email-event-storage.js +++ b/ghost/email-service/lib/email-event-storage.js @@ -18,51 +18,27 @@ class EmailEventStorage { */ listen(domainEvents) { domainEvents.subscribe(EmailDeliveredEvent, async (event) => { - try { - await this.handleDelivered(event); - } catch (err) { - logging.error(err); - } + await this.handleDelivered(event); }); domainEvents.subscribe(EmailOpenedEvent, async (event) => { - try { - await this.handleOpened(event); - } catch (err) { - logging.error(err); - } + await this.handleOpened(event); }); domainEvents.subscribe(EmailBouncedEvent, async (event) => { - try { - await this.handlePermanentFailed(event); - } catch (e) { - logging.error(e); - } + await this.handlePermanentFailed(event); }); domainEvents.subscribe(EmailTemporaryBouncedEvent, async (event) => { - try { - await this.handleTemporaryFailed(event); - } catch (e) { - logging.error(e); - } + await this.handleTemporaryFailed(event); }); domainEvents.subscribe(EmailUnsubscribedEvent, async (event) => { - try { - await this.handleUnsubscribed(event); - } catch (e) { - logging.error(e); - } + await this.handleUnsubscribed(event); }); domainEvents.subscribe(SpamComplaintEvent, async (event) => { - try { - await this.handleComplained(event); - } catch (e) { - logging.error(e); - } + await this.handleComplained(event); }); } diff --git a/ghost/email-service/test/email-event-storage.test.js b/ghost/email-service/test/email-event-storage.test.js index 8d16b7d329..42b2a16ecc 100644 --- a/ghost/email-service/test/email-event-storage.test.js +++ b/ghost/email-service/test/email-event-storage.test.js @@ -2,27 +2,20 @@ const EmailEventStorage = require('../lib/email-event-storage'); const {EmailDeliveredEvent, EmailOpenedEvent, EmailBouncedEvent, EmailTemporaryBouncedEvent, EmailUnsubscribedEvent, SpamComplaintEvent} = require('@tryghost/email-events'); const sinon = require('sinon'); const assert = require('assert'); +const logging = require('@tryghost/logging'); +const {createDb} = require('./utils'); -function stubDb() { - const db = { - knex: function () { - return this; - }, - where: function () { - return this; - }, - whereNull: function () { - return this; - }, - update: sinon.stub().resolves() - }; - db.knex.raw = function () { - return this; - }; - return db; -} +describe('Email Event Storage', function () { + let logError; + + beforeEach(function () { + logError = sinon.stub(logging, 'error'); + }); + + afterEach(function () { + sinon.restore(); + }); -describe('Email event storage', function () { describe('Constructor', function () { it('doesn\'t throw', function () { new EmailEventStorage({}); @@ -37,14 +30,15 @@ describe('Email event storage', function () { email: 'example@example.com', memberId: '123', emailId: '456', - emailRecipientId: '789' - }, new Date(0))); + emailRecipientId: '789', + timestamp: new Date(0) + })); } } }; const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); - const db = stubDb(); + const db = createDb(); const eventHandler = new EmailEventStorage({db}); eventHandler.listen(DomainEvents); sinon.assert.callCount(subscribeSpy, 6); @@ -60,14 +54,15 @@ describe('Email event storage', function () { email: 'example@example.com', memberId: '123', emailId: '456', - emailRecipientId: '789' - }, new Date(0))); + emailRecipientId: '789', + timestamp: new Date(0) + })); } } }; const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); - const db = stubDb(); + const db = createDb(); const eventHandler = new EmailEventStorage({db}); eventHandler.listen(DomainEvents); sinon.assert.callCount(subscribeSpy, 6); @@ -90,14 +85,15 @@ describe('Email event storage', function () { message: 'test', code: 500, enhancedCode: '5.5.5' - } - }, new Date(0))); + }, + timestamp: new Date(0) + })); } } }; const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); - const db = stubDb(); + const db = createDb(); const existing = { id: 1, get: (key) => { @@ -146,14 +142,15 @@ describe('Email event storage', function () { message: 'test', code: 500, enhancedCode: '5.5.5' - } - }, new Date(0))); + }, + timestamp: new Date(0) + })); } } }; const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); - const db = stubDb(); + const db = createDb(); const EmailRecipientFailure = { transaction: async function (callback) { return await callback(1); @@ -176,6 +173,37 @@ describe('Email event storage', function () { assert(EmailRecipientFailure.add.calledOnce); }); + it('Handles email permanent bounce event without error data', async function () { + let waitPromise; + + const DomainEvents = { + subscribe: async (type, handler) => { + if (type === EmailBouncedEvent) { + waitPromise = handler(EmailBouncedEvent.create({ + email: 'example@example.com', + memberId: '123', + emailId: '456', + emailRecipientId: '789', + error: null, + timestamp: new Date(0) + })); + } + } + }; + + const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); + const db = createDb(); + + const eventHandler = new EmailEventStorage({ + db, + models: {} + }); + eventHandler.listen(DomainEvents); + sinon.assert.callCount(subscribeSpy, 6); + await waitPromise; + sinon.assert.calledOnce(db.update); + }); + it('Handles email permanent bounce events with skipped update', async function () { let waitPromise; @@ -191,14 +219,15 @@ describe('Email event storage', function () { message: 'test', code: 500, enhancedCode: '5.5.5' - } - }, new Date(0))); + }, + timestamp: new Date(0) + })); } } }; const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); - const db = stubDb(); + const db = createDb(); const existing = { id: 1, get: (key) => { @@ -247,9 +276,10 @@ describe('Email event storage', function () { error: { message: 'test', code: 500, - enhancedCode: '5.5.5' - } - }, new Date(0))); + enhancedCode: null + }, + timestamp: new Date(0) + })); } } }; @@ -285,6 +315,59 @@ describe('Email event storage', function () { assert(existing.save.calledOnce); }); + it('Handles email temporary bounce events with skipped update', async function () { + let waitPromise; + + const DomainEvents = { + subscribe: async (type, handler) => { + if (type === EmailTemporaryBouncedEvent) { + waitPromise = handler(EmailTemporaryBouncedEvent.create({ + email: 'example@example.com', + memberId: '123', + emailId: '456', + emailRecipientId: '789', + error: { + message: 'test', + code: 500, + enhancedCode: '5.5.5' + }, + timestamp: new Date(0) + })); + } + } + }; + + const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); + const existing = { + id: 1, + get: (key) => { + if (key === 'severity') { + return 'temporary'; + } + if (key === 'failed_at') { + return new Date(5); + } + }, + save: sinon.stub().resolves() + }; + const EmailRecipientFailure = { + transaction: async function (callback) { + return await callback(1); + }, + findOne: sinon.stub().resolves(existing) + }; + + const eventHandler = new EmailEventStorage({ + models: { + EmailRecipientFailure + } + }); + eventHandler.listen(DomainEvents); + sinon.assert.callCount(subscribeSpy, 6); + await waitPromise; + assert(existing.save.notCalled); + }); + it('Handles unsubscribe', async function () { let waitPromise; @@ -294,8 +377,9 @@ describe('Email event storage', function () { waitPromise = handler(EmailUnsubscribedEvent.create({ email: 'example@example.com', memberId: '123', - emailId: '456' - }, new Date(0))); + emailId: '456', + timestamp: new Date(0) + })); } } }; @@ -324,8 +408,9 @@ describe('Email event storage', function () { waitPromise = handler(SpamComplaintEvent.create({ email: 'example@example.com', memberId: '123', - emailId: '456' - }, new Date(0))); + emailId: '456', + timestamp: new Date(0) + })); } } }; @@ -345,4 +430,70 @@ describe('Email event storage', function () { await waitPromise; assert(EmailSpamComplaintEvent.add.calledOnce); }); + + it('Handles duplicate complaints', async function () { + let waitPromise; + + const DomainEvents = { + subscribe: async (type, handler) => { + if (type === SpamComplaintEvent) { + waitPromise = handler(SpamComplaintEvent.create({ + email: 'example@example.com', + memberId: '123', + emailId: '456', + timestamp: new Date(0) + })); + } + } + }; + + const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); + const EmailSpamComplaintEvent = { + add: sinon.stub().rejects({code: 'ER_DUP_ENTRY'}) + }; + + const eventHandler = new EmailEventStorage({ + models: { + EmailSpamComplaintEvent + } + }); + eventHandler.listen(DomainEvents); + sinon.assert.callCount(subscribeSpy, 6); + await waitPromise; + assert(EmailSpamComplaintEvent.add.calledOnce); + assert(!logError.calledOnce); + }); + + it('Handles logging failed complaint storage', async function () { + let waitPromise; + + const DomainEvents = { + subscribe: async (type, handler) => { + if (type === SpamComplaintEvent) { + waitPromise = handler(SpamComplaintEvent.create({ + email: 'example@example.com', + memberId: '123', + emailId: '456', + timestamp: new Date(0) + })); + } + } + }; + + const subscribeSpy = sinon.spy(DomainEvents, 'subscribe'); + const EmailSpamComplaintEvent = { + add: sinon.stub().rejects(new Error('Some database error')) + }; + + const eventHandler = new EmailEventStorage({ + models: { + EmailSpamComplaintEvent + } + }); + eventHandler.listen(DomainEvents); + sinon.assert.callCount(subscribeSpy, 6); + await waitPromise; + assert(EmailSpamComplaintEvent.add.calledOnce); + assert(logError.calledOnce); + }); }); diff --git a/ghost/email-service/test/utils/index.js b/ghost/email-service/test/utils/index.js index a6ac505abd..d2ad6bb6b8 100644 --- a/ghost/email-service/test/utils/index.js +++ b/ghost/email-service/test/utils/index.js @@ -59,7 +59,7 @@ const createModelClass = (options = {}) => { }; }; -const createDb = ({first, all}) => { +const createDb = ({first, all} = {}) => { let a = all; const db = { knex: function () {