From 42f9d392a313ee14e260f495877a1453c61830ad Mon Sep 17 00:00:00 2001 From: Rishabh Garg Date: Wed, 30 Nov 2022 16:21:58 +0530 Subject: [PATCH] Added mailgun provider for sending emails (#15896) closes https://github.com/TryGhost/Team/issues/2309 - adds new mailgun provider to send out batch emails - updates sending service to send email id for mailgun provider, allows tagging mail with email id --- .../server/services/email-service/wrapper.js | 50 +++--- ghost/email-service/index.js | 3 +- .../lib/batch-sending-service.js | 26 +-- ghost/email-service/lib/email-renderer.js | 38 ++-- ghost/email-service/lib/email-service.js | 9 +- .../lib/mailgun-email-provider.js | 168 ++++++++++++++++++ ghost/email-service/lib/sending-service.js | 21 ++- .../test/mailgun-email-provider.test.js | 131 ++++++++++++++ 8 files changed, 375 insertions(+), 71 deletions(-) create mode 100644 ghost/email-service/lib/mailgun-email-provider.js create mode 100644 ghost/email-service/test/mailgun-email-provider.test.js diff --git a/ghost/core/core/server/services/email-service/wrapper.js b/ghost/core/core/server/services/email-service/wrapper.js index 84db4a708f..d2fe63e9d0 100644 --- a/ghost/core/core/server/services/email-service/wrapper.js +++ b/ghost/core/core/server/services/email-service/wrapper.js @@ -1,5 +1,4 @@ const logging = require('@tryghost/logging'); -const ObjectID = require('bson-objectid').default; const url = require('../../../server/api/endpoints/utils/serializers/output/utils/url'); class EmailServiceWrapper { @@ -14,13 +13,16 @@ class EmailServiceWrapper { return; } - const {EmailService, EmailController, EmailRenderer, SendingService, BatchSendingService, EmailSegmenter, EmailEventStorage} = require('@tryghost/email-service'); + const {EmailService, EmailController, EmailRenderer, SendingService, BatchSendingService, EmailSegmenter, EmailEventStorage, MailgunEmailProvider} = require('@tryghost/email-service'); const {Post, Newsletter, Email, EmailBatch, EmailRecipient, Member} = require('../../models'); + const MailgunClient = require('@tryghost/mailgun-client'); + const configService = require('../../../shared/config'); const settingsCache = require('../../../shared/settings-cache'); const settingsHelpers = require('../../services/settings-helpers'); const jobsService = require('../jobs'); const membersService = require('../members'); const db = require('../../data/db'); + const sentry = require('../../../shared/sentry'); const membersRepository = membersService.api.members; const limitService = require('../limits'); const domainEvents = require('@tryghost/domain-events'); @@ -33,6 +35,22 @@ class EmailServiceWrapper { const linkTracking = require('../link-tracking'); const audienceFeedback = require('../audience-feedback'); + // capture errors from mailgun client and log them in sentry + const errorHandler = (error) => { + logging.info(`Capturing error for mailgun email provider service`); + sentry.captureException(error); + }; + + // Mailgun client instance for email provider + const mailgunClient = new MailgunClient({ + config: configService, settings: settingsCache + }); + + const mailgunEmailProvider = new MailgunEmailProvider({ + mailgunClient, + errorHandler + }); + const emailRenderer = new EmailRenderer({ settingsCache, settingsHelpers, @@ -50,31 +68,7 @@ class EmailServiceWrapper { }); const sendingService = new SendingService({ - emailProvider: { - send: async ({plaintext, subject, from, replyTo, recipients}) => { - logging.info(`Sending email\nSubject: ${subject}\nFrom: ${from}\nReplyTo: ${replyTo}\nRecipients: ${recipients.length}\n\n${plaintext}`); - - // Uncomment to test email HTML rendering with GhostMailer - /*const {GhostMailer} = require('../mail'); - const mailer = new GhostMailer(); - logging.info(`Sending email\nSubject: ${subject}\nFrom: ${from}\nReplyTo: ${replyTo}\nRecipients: ${recipients.length}\n\n${JSON.stringify(recipients[0].replacements, undefined, ' ')}`); - - for (const replacement of recipients[0].replacements) { - html = html.replace(replacement.token, replacement.value); - plaintext = plaintext.replace(replacement.token, replacement.value); - } - - await mailer.send({ - subject, - html, - to: recipients[0].email, - from, - replyTo, - text: plaintext - });*/ - return Promise.resolve({id: 'fake_provider_id_' + ObjectID().toHexString()}); - } - }, + emailProvider: mailgunEmailProvider, emailRenderer }); @@ -105,7 +99,7 @@ class EmailServiceWrapper { emailSegmenter, limitService }); - + this.controller = new EmailController(this.service, { models: { Post, diff --git a/ghost/email-service/index.js b/ghost/email-service/index.js index 31ff58c747..1a999bf777 100644 --- a/ghost/email-service/index.js +++ b/ghost/email-service/index.js @@ -6,5 +6,6 @@ module.exports = { SendingService: require('./lib/sending-service'), BatchSendingService: require('./lib/batch-sending-service'), EmailEventProcessor: require('./lib/email-event-processor'), - EmailEventStorage: require('./lib/email-event-storage') + EmailEventStorage: require('./lib/email-event-storage'), + MailgunEmailProvider: require('./lib/mailgun-email-provider') }; diff --git a/ghost/email-service/lib/batch-sending-service.js b/ghost/email-service/lib/batch-sending-service.js index acc751af4e..845f4267f7 100644 --- a/ghost/email-service/lib/batch-sending-service.js +++ b/ghost/email-service/lib/batch-sending-service.js @@ -29,7 +29,7 @@ class BatchSendingService { #db; /** - * @param {Object} dependencies + * @param {Object} dependencies * @param {EmailRenderer} dependencies.emailRenderer * @param {SendingService} dependencies.sendingService * @param {JobsService} dependencies.jobsService @@ -59,7 +59,7 @@ class BatchSendingService { /** * Schedules a background job that sends the email in the background if it is pending or failed. - * @param {Email} email + * @param {Email} email * @returns {void} */ scheduleEmail(email) { @@ -106,7 +106,7 @@ class BatchSendingService { /** * @private - * @param {Email} email + * @param {Email} email * @throws {errors.EmailError} If one of the batches fails */ async sendEmail(email) { @@ -267,8 +267,8 @@ class BatchSendingService { } /** - * - * @param {{email: Email, batch: EmailBatch, post: Post, newsletter: Newsletter}} data + * + * @param {{email: Email, batch: EmailBatch, post: Post, newsletter: Newsletter}} data * @returns {Promise} True when succeeded, false when failed with an error */ async sendBatch({email, batch, post, newsletter}) { @@ -286,6 +286,7 @@ class BatchSendingService { try { const members = await this.getBatchMembers(batch.id); const response = await this.#sendingService.send({ + emailId: email.id, post, newsletter, segment: batch.get('member_segment'), @@ -297,18 +298,21 @@ class BatchSendingService { await batch.save({ status: 'submitted', - provider_id: response.id + provider_id: response.id, + // reset error fields when sending succeeds + error_status_code: null, + error_message: null, + error_data: null }, {patch: true, require: false}); succeeded = true; } catch (err) { logging.error(`Error sending email batch ${batch.id}`, err); await batch.save({ - status: 'failed' - // TODO: error should be instance of EmailProviderError (see IEmailProviderService) + we should read error message - // error_status_code: err.status_code, - // error_message: err.message_short, - // error_data: err.message_full + status: 'failed', + error_status_code: err.statusCode, + error_message: err.message, + error_data: err.errorDetails }, {patch: true, require: false}); } diff --git a/ghost/email-service/lib/email-renderer.js b/ghost/email-service/lib/email-renderer.js index 43394bfc81..dcb0f45bcc 100644 --- a/ghost/email-service/lib/email-renderer.js +++ b/ghost/email-service/lib/email-renderer.js @@ -144,21 +144,21 @@ class EmailRenderer { getSegments(post) { const allowedSegments = ['status:free', 'status:-free']; const html = this.renderPostBaseHtml(post); - + const cheerio = require('cheerio'); const $ = cheerio.load(html); - + let allSegments = $('[data-gh-segment]') .get() .map(el => el.attribs['data-gh-segment']); - + /** * Always add free and paid segments if email has paywall card */ if (html.indexOf('') !== -1) { allSegments = allSegments.concat(['status:free', 'status:-free']); } - + const segments = [...new Set(allSegments)].filter(segment => allowedSegments.includes(segment)); if (segments.length === 0) { // One segment to all members @@ -182,11 +182,11 @@ class EmailRenderer { } /** - * - * @param {Post} post - * @param {Newsletter} newsletter - * @param {Segment} segment - * @param {EmailRenderOptions} options + * + * @param {Post} post + * @param {Newsletter} newsletter + * @param {Segment} segment + * @param {EmailRenderOptions} options * @returns {Promise} */ async renderBody(post, newsletter, segment, options) { @@ -202,15 +202,15 @@ class EmailRenderer { if (segment === 'status:free') { // Add paywall addPaywall = true; - + // Remove the members-only content html = html.slice(0, membersOnlyIndex); } } const templateData = await this.getTemplateData({ - post, - newsletter, + post, + newsletter, html, addPaywall }); @@ -258,14 +258,14 @@ class EmailRenderer { // force all links to open in new tab $('a').attr('target', '_blank'); - + // convert figure and figcaption to div so that Outlook applies margins $('figure, figcaption').each((i, elem) => !!(elem.tagName = 'div')); - + // Remove/hide parts of the email based on segment data attributes $('[data-gh-segment]').get().forEach((node) => { // TODO: replace with NQL interpretation - if (node.attribs['data-gh-segment'] !== segment) { + if (node.attribs['data-gh-segment'] !== segment) { $(node).remove(); } else { // Getting rid of the attribute for a cleaner html output @@ -274,7 +274,7 @@ class EmailRenderer { }); // Convert DOM back to HTML - html = $.html(); // () Fix for vscode syntax highlighter + html = $.html(); // () Fix for vscode syntax highlighter // Replacement strings const replacementDefinitions = this.buildReplacementDefinitions({html, newsletter}); @@ -351,7 +351,7 @@ class EmailRenderer { { id: 'first_name', getValue: (member) => { - return member.name.split(' ')[0]; + return member.name?.split(' ')[0]; } } ]; @@ -515,7 +515,7 @@ class EmailRenderer { site: { title: this.#settingsCache.get('title'), url: this.#urlUtils.urlFor('home', true), - iconUrl: this.#settingsCache.get('icon') ? + iconUrl: this.#settingsCache.get('icon') ? this.#urlUtils.urlFor('image', { image: this.#settingsCache.get('icon') }, true) : null @@ -570,7 +570,7 @@ class EmailRenderer { width: 100, height: 38, iconWidth: 24 - }, + }, // Sizes defined in pixels won’t be adjusted when Outlook is rendering at 120 dpi. // To solve the problem we use values in points (1 pixel = 0.75 point). // resource: https://www.hteumeuleu.com/2021/background-properties-in-vml/ diff --git a/ghost/email-service/lib/email-service.js b/ghost/email-service/lib/email-service.js index 4dc37f2bd0..a8416683de 100644 --- a/ghost/email-service/lib/email-service.js +++ b/ghost/email-service/lib/email-service.js @@ -11,6 +11,7 @@ const errors = require('@tryghost/errors'); const tpl = require('@tryghost/tpl'); const EmailRenderer = require('./email-renderer'); const EmailSegmenter = require('./email-segmenter'); +const MailgunEmailProvider = require('./mailgun-email-provider'); const messages = { archivedNewsletterError: 'Cannot send email to archived newsletters', @@ -26,8 +27,8 @@ class EmailService { #limitService; /** - * - * @param {object} dependencies + * + * @param {object} dependencies * @param {BatchSendingService} dependencies.batchSendingService * @param {object} dependencies.models * @param {object} dependencies.models.Email @@ -69,8 +70,8 @@ class EmailService { } /** - * - * @param {Post} post + * + * @param {Post} post * @returns {Promise} */ async createEmail(post) { diff --git a/ghost/email-service/lib/mailgun-email-provider.js b/ghost/email-service/lib/mailgun-email-provider.js new file mode 100644 index 0000000000..3109c607a5 --- /dev/null +++ b/ghost/email-service/lib/mailgun-email-provider.js @@ -0,0 +1,168 @@ +const logging = require('@tryghost/logging'); +const errors = require('@tryghost/errors'); +const debug = require('@tryghost/debug')('email-service:mailgun-provider-service'); + +/** + * @typedef {object} Recipient + * @prop {string} email + * @prop {Replacement[]} replacements + */ + +/** + * @typedef {object} Replacement + * @prop {string} token + * @prop {string} value + * @prop {string} id + */ + +/** + * @typedef {object} EmailSendingOptions + * @prop {boolean} clickTrackingEnabled + * @prop {boolean} openTrackingEnabled + */ + +/** + * @typedef {object} EmailProviderSuccessResponse + * @prop {string} id + */ + +class MailgunEmailProvider { + #mailgunClient; + #errorHandler; + + static BATCH_SIZE = 1000; + + /** + * @param {object} dependencies + * @param {import('@tryghost/mailgun-client/lib/mailgun-client')} dependencies.mailgunClient - mailgun client to send emails + * @param {Function} [dependencies.errorHandler] - custom error handler for logging exceptions + */ + constructor({ + mailgunClient, + errorHandler + }) { + this.#mailgunClient = mailgunClient; + this.#errorHandler = errorHandler; + } + + #createRecipientData(replacements) { + let recipientData = {}; + + recipientData = replacements.reduce((acc, replacement) => { + const {id, value} = replacement; + if (!acc[id]) { + acc[id] = {}; + } + acc[id] = value; + return acc; + }, {}); + + return recipientData; + } + + #updateRecipientVariables(data, replacementDefinitions) { + for (const def of replacementDefinitions) { + data = data.replace( + def.token, + `%recipient.${def.id}%` + ); + } + return data; + } + + /** + * Create mailgun error message for storing in the database + * @param {Object} error + * @param {string} error.message + * @param {string} error.details + * @returns {string} + */ + #createMailgunErrorMessage(error) { + const message = (error?.message || '') + ':' + (error?.details || ''); + return message.slice(0, 2000); + } + + /** + * Send an email using the Mailgun API + * @param {import('./sending-service').EmailData} data + * @param {EmailSendingOptions} options + * @returns {Promise} + */ + async send(data, options) { + const { + subject, + html, + plaintext, + from, + replyTo, + emailId, + recipients, + replacementDefinitions + } = data; + + logging.info(`Sending email to ${recipients.length} recipients`); + const startTime = Date.now(); + debug(`sending message to ${recipients.length} recipients`); + + try { + const messageData = { + subject, + html, + plaintext, + from, + replyTo, + id: emailId, + track_opens: !!options.openTrackingEnabled, + track_clicks: !!options.clickTrackingEnabled + }; + + // create recipient data for Mailgun using replacement definitions + const recipientData = recipients.reduce((acc, recipient) => { + acc[recipient.email] = this.#createRecipientData(recipient.replacements); + return acc; + }, {}); + + // update content to use Mailgun variable syntax for all replacements + ['html', 'plaintext'].forEach((key) => { + if (messageData[key]) { + messageData[key] = this.#updateRecipientVariables(messageData[key], replacementDefinitions); + } + }); + + // send the email using Mailgun + // uses empty replacements array as we've already replaced all tokens with Mailgun variables + const response = await this.#mailgunClient.send( + messageData, + recipientData, + [] + ); + + debug(`sent message (${Date.now() - startTime}ms)`); + logging.info(`Sent message (${Date.now() - startTime}ms)`); + + // Return mailgun provider id, trim <> from response + return { + id: response.id.trim().replace(/^<|>$/g, '') + }; + } catch ({error, messageData}) { + // REF: possible mailgun errors https://documentation.mailgun.com/en/latest/api-intro.html#status-codes + let ghostError = new errors.EmailError({ + statusCode: error.status, + message: this.#createMailgunErrorMessage(error), + errorDetails: JSON.stringify({error, messageData}), + context: `Mailgun Error ${error.status}: ${error.details}`, + help: `https://ghost.org/docs/newsletters/#bulk-email-configuration`, + code: 'BULK_EMAIL_SEND_FAILED' + }); + + logging.warn(ghostError); + debug(`failed to send message (${Date.now() - startTime}ms)`); + + // log error to custom error handler. ex sentry + this.#errorHandler(ghostError); + throw ghostError; + } + } +} + +module.exports = MailgunEmailProvider; diff --git a/ghost/email-service/lib/sending-service.js b/ghost/email-service/lib/sending-service.js index 42b377bbb6..6a30782982 100644 --- a/ghost/email-service/lib/sending-service.js +++ b/ghost/email-service/lib/sending-service.js @@ -4,12 +4,14 @@ * @prop {string} plaintext * @prop {string} subject * @prop {string} from + * @prop {string} emailId * @prop {string} [replyTo] * @prop {Recipient[]} recipients - * + * @prop {import("./email-renderer").ReplacementDefinition[]} replacementDefinitions + * * @typedef {object} IEmailProviderService * @prop {(emailData: EmailData, options: EmailSendingOptions) => Promise} send - * + * * @typedef {object} Post * @typedef {object} Newsletter */ @@ -63,20 +65,21 @@ class SendingService { this.#emailRenderer = emailRenderer; } - /** + /** * Send a given post, rendered for a given newsletter and segment to the members provided in the list * @param {object} data * @param {Post} data.post * @param {Newsletter} data.newsletter * @param {string|null} data.segment + * @param {string|null} data.emailId * @param {MemberLike[]} data.members * @param {EmailSendingOptions} options * @returns {Promise} */ - async send({post, newsletter, segment, members}, options) { + async send({post, newsletter, segment, members, emailId}, options) { const emailBody = await this.#emailRenderer.renderBody( - post, - newsletter, + post, + newsletter, segment, options ); @@ -88,10 +91,12 @@ class SendingService { replyTo: this.#emailRenderer.getReplyToAddress(post, newsletter) ?? undefined, html: emailBody.html, plaintext: emailBody.plaintext, - recipients + recipients, + emailId: emailId, + replacementDefinitions: emailBody.replacements }, options); } - + /** * @private * @param {MemberLike[]} members diff --git a/ghost/email-service/test/mailgun-email-provider.test.js b/ghost/email-service/test/mailgun-email-provider.test.js new file mode 100644 index 0000000000..841a582927 --- /dev/null +++ b/ghost/email-service/test/mailgun-email-provider.test.js @@ -0,0 +1,131 @@ +const MailgunEmailProvider = require('../lib/mailgun-email-provider'); +const sinon = require('sinon'); +const should = require('should'); + +describe('Mailgun Email Provider', function () { + describe('send', function () { + let mailgunClient; + let sendStub; + + beforeEach(function () { + sendStub = sinon.stub().resolves({ + id: 'provider-123' + }); + + mailgunClient = { + send: sendStub + }; + }); + + afterEach(function () { + sinon.restore(); + }); + + it('calls mailgun client with correct data', async function () { + const mailgunEmailProvider = new MailgunEmailProvider({ + mailgunClient, + errorHandler: () => {} + }); + + const response = await mailgunEmailProvider.send({ + subject: 'Hi', + html: 'Hi {{name}}', + plaintext: 'Hi', + from: 'ghost@example.com', + replyTo: 'ghost@example.com', + emailId: '123', + recipients: [ + { + email: 'member@example.com', + replacements: [ + { + id: 'name', + token: '{{name}}', + value: 'John' + } + ] + } + ], + replacementDefinitions: [ + { + id: 'name', + token: '{{name}}', + getValue: () => 'John' + } + ] + }, { + clickTrackingEnabled: true, + openTrackingEnabled: true + }); + should(response.id).eql('provider-123'); + should(sendStub.calledOnce).be.true(); + sendStub.calledWith( + { + subject: 'Hi', + html: 'Hi %recipient.name%', + plaintext: 'Hi', + from: 'ghost@example.com', + replyTo: 'ghost@example.com', + id: '123', + track_opens: true, + track_clicks: true + }, + {'member@example.com': {name: 'John'}}, + [] + ).should.be.true(); + }); + + it('handles mailgun client error correctly', async function () { + const mailgunErr = new Error('Bad Request'); + mailgunErr.details = 'Invalid domain'; + mailgunErr.status = 400; + sendStub = sinon.stub().throws({ + error: mailgunErr, + messageData: {} + }); + + mailgunClient = { + send: sendStub + }; + + const mailgunEmailProvider = new MailgunEmailProvider({ + mailgunClient, + errorHandler: () => {} + }); + try { + const response = await mailgunEmailProvider.send({ + subject: 'Hi', + html: 'Hi {{name}}', + plaintext: 'Hi', + from: 'ghost@example.com', + replyTo: 'ghost@example.com', + emailId: '123', + recipients: [ + { + email: 'member@example.com', + replacements: [ + { + id: 'name', + token: '{{name}}', + value: 'John' + } + ] + } + ], + replacementDefinitions: [ + { + id: 'name', + token: '{{name}}', + getValue: () => 'John' + } + ] + }, {}); + should(response).be.undefined(); + } catch (e) { + should(e.message).eql('Bad Request:Invalid domain'); + should(e.statusCode).eql(400); + should(e.errorDetails).eql('{"error":{"details":"Invalid domain","status":400},"messageData":{}}'); + } + }); + }); +});