🐛 Handled unknown Mailgun events (#15995)

refs https://ghost.slack.com/archives/C02G9E68C/p1670916538764019

- We receive events that don't have an emailId or providerId.
- We filter those events now and log them as an error

ghost/core/test/integration/services/email-service/email-event-storage.test.js

@@ -35,7 +35,7 @@ describe('EmailEventStorage', function () {
         jobsService = require('../../../../core/server/services/jobs');
 
         sinon.stub(MailgunClient.prototype, 'fetchEvents').callsFake(async function (_, batchHandler) {
-            const normalizedEvents = events.map(this.normalizeEvent) || [];
+            const normalizedEvents = (events.map(this.normalizeEvent) || []).filter(e => !!e);
             return [await batchHandler(normalizedEvents)];
         });
     });
@@ -875,4 +875,23 @@ describe('EmailEventStorage', function () {
         assert.deepEqual(result.emailIds, []);
         assert.deepEqual(result.memberIds, []);
     });
+
+    it('Ignores invalid events', async function () {
+        const emailBatch = fixtureManager.get('email_batches', 0);
+        const emailRecipient = fixtureManager.get('email_recipients', 0);
+        assert(emailRecipient.batch_id === emailBatch.id);
+
+        events = [{
+            event: 'ceci-nest-pas-un-event'
+        }];
+
+        // Fire event processing
+        // We use offloading to have correct coverage and usage of worker thread
+        const {eventStats: result} = await run({
+            domainEvents
+        });
+        assert.equal(result.unhandled, 0);
+        assert.deepEqual(result.emailIds, []);
+        assert.deepEqual(result.memberIds, []);
+    });
 });

ghost/email-analytics-service/lib/email-analytics-service.js

@@ -13,7 +13,7 @@ module.exports = class EmailAnalyticsService {
     providers;
 
     /**
-     * @param {object} dependencies 
+     * @param {object} dependencies
      * @param {EmailEventProcessor} dependencies.eventProcessor
      */
     constructor({config, settings, queries, eventProcessor, providers}) {
@@ -83,8 +83,8 @@ module.exports = class EmailAnalyticsService {
     }
 
     /**
-     * 
-     * @param {{id: string, type: any; severity: any; recipientEmail: any; emailId: any; providerId: string; timestamp: Date; error: {code: number; message: string; enhandedCode: string|number} | null}} event 
+     *
+     * @param {{id: string, type: any; severity: any; recipientEmail: any; emailId: any; providerId: string; timestamp: Date; error: {code: number; message: string; enhandedCode: string|number} | null}} event
      * @returns {Promise<EventProcessingResult>}
      */
     async processEvent(event) {

ghost/email-service/lib/email-event-processor.js

@@ -30,8 +30,8 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {Date} timestamp 
+     * @param {EmailIdentification} emailIdentification
+     * @param {Date} timestamp
      */
     async handleDelivered(emailIdentification, timestamp) {
         const recipient = await this.getRecipient(emailIdentification);
@@ -48,8 +48,8 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {Date} timestamp 
+     * @param {EmailIdentification} emailIdentification
+     * @param {Date} timestamp
      */
     async handleOpened(emailIdentification, timestamp) {
         const recipient = await this.getRecipient(emailIdentification);
@@ -66,14 +66,14 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {{id: string, timestamp: Date, error: {code: number; message: string; enhandedCode: string|number} | null}} event 
+     * @param {EmailIdentification} emailIdentification
+     * @param {{id: string, timestamp: Date, error: {code: number; message: string; enhandedCode: string|number} | null}} event
      */
     async handleTemporaryFailed(emailIdentification, {timestamp, error, id}) {
         const recipient = await this.getRecipient(emailIdentification);
         if (recipient) {
             this.#domainEvents.dispatch(EmailTemporaryBouncedEvent.create({
-                id, 
+                id,
                 error,
                 email: emailIdentification.email,
                 memberId: recipient.memberId,
@@ -86,8 +86,8 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {{id: string, timestamp: Date, error: {code: number; message: string; enhandedCode: string|number} | null}} event 
+     * @param {EmailIdentification} emailIdentification
+     * @param {{id: string, timestamp: Date, error: {code: number; message: string; enhandedCode: string|number} | null}} event
      */
     async handlePermanentFailed(emailIdentification, {timestamp, error, id}) {
         const recipient = await this.getRecipient(emailIdentification);
@@ -106,8 +106,8 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {Date} timestamp 
+     * @param {EmailIdentification} emailIdentification
+     * @param {Date} timestamp
      */
     async handleUnsubscribed(emailIdentification, timestamp) {
         const recipient = await this.getRecipient(emailIdentification);
@@ -123,8 +123,8 @@ class EmailEventProcessor {
     }
 
     /**
-     * @param {EmailIdentification} emailIdentification 
-     * @param {Date} timestamp 
+     * @param {EmailIdentification} emailIdentification
+     * @param {Date} timestamp
      */
     async handleComplained(emailIdentification, timestamp) {
         const recipient = await this.getRecipient(emailIdentification);
@@ -141,10 +141,15 @@ class EmailEventProcessor {
 
     /**
      * @private
-     * @param {EmailIdentification} emailIdentification 
+     * @param {EmailIdentification} emailIdentification
      * @returns {Promise<EmailRecipientInformation|undefined>}
      */
     async getRecipient(emailIdentification) {
+        if (!emailIdentification.emailId && !emailIdentification.providerId) {
+            // Protection if both are null or undefined
+            return;
+        }
+
         // With the provider_id and email address we can look for the EmailRecipient
         const emailId = emailIdentification.emailId ?? await this.getEmailId(emailIdentification.providerId);
         if (!emailId) {
@@ -157,7 +162,7 @@ class EmailEventProcessor {
             .where('member_email', emailIdentification.email)
             .where('email_id', emailId)
             .first() || {};
-        
+
         if (emailRecipientId && memberId) {
             return {
                 emailRecipientId,
@@ -169,7 +174,7 @@ class EmailEventProcessor {
 
     /**
      * @private
-     * @param {string} providerId 
+     * @param {string} providerId
      * @returns {Promise<string|undefined>}
      */
     async getEmailId(providerId) {

ghost/email-service/lib/email-service.js

@@ -169,10 +169,10 @@ class EmailService {
     }
 
     /**
-     * 
-     * @param {*} post 
-     * @param {*} newsletter 
-     * @param {import('./email-renderer').Segment} segment 
+     *
+     * @param {*} post
+     * @param {*} newsletter
+     * @param {import('./email-renderer').Segment} segment
      * @returns {Promise<{subject: string, html: string, plaintext: string}>} Email preview
      */
     async previewEmail(post, newsletter, segment) {
@@ -195,10 +195,10 @@ class EmailService {
     }
 
     /**
-     * 
-     * @param {*} post 
-     * @param {*} newsletter 
-     * @param {import('./email-renderer').Segment} segment 
+     *
+     * @param {*} post
+     * @param {*} newsletter
+     * @param {import('./email-renderer').Segment} segment
      * @param {string[]} emails
      */
     async sendTestEmail(post, newsletter, segment, emails) {

ghost/mailgun-client/lib/mailgun-client.js

@@ -125,7 +125,7 @@ module.exports = class MailgunClient {
                 value: Date.now() - startTime,
                 statusCode: 200
             });
-            let events = page?.items?.map(this.normalizeEvent) || [];
+            let events = (page?.items?.map(this.normalizeEvent) || []).filter(e => !!e);
             debug(`fetchEvents: finished fetching first page with ${events.length} events`);
 
             let eventCount = 0;
@@ -152,7 +152,7 @@ module.exports = class MailgunClient {
                     value: Date.now() - startTime,
                     statusCode: 200
                 });
-                events = page?.items?.map(this.normalizeEvent) || [];
+                events = (page?.items?.map(this.normalizeEvent) || []).filter(e => !!e);
                 debug(`fetchEvents: finished fetching next page with ${events.length} events`);
             }
 
@@ -203,6 +203,12 @@ module.exports = class MailgunClient {
     normalizeEvent(event) {
         const providerId = event?.message?.headers['message-id'];
 
+        if (!providerId && !(event['user-variables'] && event['user-variables']['email-id'])) {
+            logging.error('Received invalid event from Mailgun');
+            logging.error(event);
+            return null;
+        }
+
         return {
             id: event.id,
             type: event.event,