TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-27 10:42:45 +03:00
Code Issues Projects Releases Wiki Activity

Improved mega error handling (#11393)

Browse Source 
no issue

- Increased default mailgun retry limit to 5
- Handling retry logic closer to SDK layer gives less future manual handling
- Allowed failing request to be passed through to the caller
- To be able to handle failed requests more gracefully in the future we need all available error information to be given to the caller
- The previous method with `Promise.all` would have rejected a whole batch without providing details on each specific batch.
- Limited data returned with a failed message to batch values
- Added better error handling on mega layer
- Added new column to store failed batch info
- Added reference to mailgan error docs
- Refactored batch emailer to respond with instances of an object
- It's hard to reason about the response type of bulk mailer when multiple object types can be returned
- This gives more clarity and ability to check with `instanceof` check
This commit is contained in:
Naz Gargol 2019-11-15 18:25:33 +07:00 committed by GitHub
parent ee47dd4dae
commit c99f40957e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 116 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
core/server/data/schema/schema.js
View File

@ -389,6 +389,7 @@ module.exports = {
validations: {isIn: [['pending', 'submitting', 'submitted', 'failed']]}
},
error: {type: 'string', maxlength: 2000, nullable: true},
error_data: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true},
meta: {type: 'text', maxlength: 65535, nullable: true},
stats: {type: 'text', maxlength: 65535, nullable: true},
email_count: {type: 'integer', nullable: false, unsigned: true, defaultTo: 0},

94
core/server/services/bulk-email/index.js
View File

@ -4,6 +4,29 @@ const mailgunProvider = require('./mailgun');
const configService = require('../../config');
const settingsCache = require('../settings/cache');
/**
* An object representing batch request result
* @typedef { Object } BatchResultBase
* @property { string } data - data that is returned from Mailgun or one which Mailgun was called with
*/
class BatchResultBase {
}
class SuccessfulBatch extends BatchResultBase {
constructor(data) {
super();
this.data = data;
}
}
class FailedBatch extends BatchResultBase {
constructor(error, data) {
super();
this.error = error;
this.data = data;
}
}
/**
* An email address
* @typedef { string } EmailAddress
@ -17,11 +40,13 @@ const settingsCache = require('../settings/cache');
*/
module.exports = {
SuccessfulBatch,
FailedBatch,
/**
* @param {Email} message - The message to send
* @param {[EmailAddress]} recipients - the recipients to send the email to
* @param {[object]} recipientData - list of data keyed by email to inject into the email
* @returns {Promise<Array<object>>} An array of promises representing the success of the batch email sending
* @returns {Promise<Array<BatchResultBase>>} An array of promises representing the success of the batch email sending
*/
async send(message, recipients, recipientData = {}) {
let BATCH_SIZE = 1000;
@ -36,33 +61,52 @@ module.exports = {
BATCH_SIZE = 2;
}
try {
const chunkedRecipients = _.chunk(recipients, BATCH_SIZE);
const blogTitle = settingsCache.get('title');
fromAddress = blogTitle ? `${blogTitle}<${fromAddress}>` : fromAddress;
return Promise.map(chunkedRecipients, (toAddresses) => {
const recipientVariables = {};
toAddresses.forEach((email) => {
recipientVariables[email] = recipientData[email];
});
const messageData = Object.assign({}, message, {
to: toAddresses,
from: fromAddress,
'recipient-variables': recipientVariables
});
const bulkEmailConfig = configService.get('bulkEmail');
const blogTitle = settingsCache.get('title');
fromAddress = blogTitle ? `${blogTitle}<${fromAddress}>` : fromAddress;
if (bulkEmailConfig && bulkEmailConfig.mailgun && bulkEmailConfig.mailgun.tag) {
Object.assign(messageData, {
'o:tag': [bulkEmailConfig.mailgun.tag, 'bulk-email']
});
}
const chunkedRecipients = _.chunk(recipients, BATCH_SIZE);
return mailgunInstance.messages().send(messageData);
return Promise.mapSeries(chunkedRecipients, (toAddresses) => {
const recipientVariables = {};
toAddresses.forEach((email) => {
recipientVariables[email] = recipientData[email];
});
} catch (err) {
common.logging.error({err});
}
const batchData = {
to: toAddresses,
from: fromAddress,
'recipient-variables': recipientVariables
};
const bulkEmailConfig = configService.get('bulkEmail');
if (bulkEmailConfig && bulkEmailConfig.mailgun && bulkEmailConfig.mailgun.tag) {
Object.assign(batchData, {
'o:tag': [bulkEmailConfig.mailgun.tag, 'bulk-email']
});
}
const messageData = Object.assign({}, message, batchData);
return new Promise((resolve) => {
mailgunInstance.messages().send(messageData, (error, body) => {
if (error) {
// NOTE: logging an error here only but actual handling should happen in more sophisticated batch retry handler
// REF: possible mailgun errors https://documentation.mailgun.com/en/latest/api-intro.html#errors
common.logging.warn(new common.errors.GhostError({
err: error,
context: common.i18n.t('errors.services.mega.requestFailed.error')
}));
// NOTE: these are generated variables, so can be regenerated when retry is done
const data = _.omit(batchData, ['recipient-variables']);
resolve(new FailedBatch(error, data));
} else {
resolve(new SuccessfulBatch(body));
}
});
});
});
}
};

3
core/server/services/bulk-email/mailgun.js
View File

@ -13,7 +13,8 @@ function createMailgun(config) {
protocol: baseUrl.protocol,
host: baseUrl.host,
port: baseUrl.port,
endpoint: baseUrl.pathname
endpoint: baseUrl.pathname,
retry: 5
});
}

46
core/server/services/mega/mega.js
View File

@ -186,14 +186,46 @@ async function listener(emailModel, options) {
id: emailModel.id
});
const meta = await sendEmail(post, members);
let meta = [];
let error;
await models.Email.edit({
status: 'submitted',
meta: JSON.stringify(meta)
}, {
id: emailModel.id
});
try {
// NOTE: meta can contains an array which can be a mix of successful and error responses
// needs filtering and saving objects of {error, batchData} form to separate property
meta = await sendEmail(post, members);
} catch (err) {
common.logging.error(new common.errors.GhostError({
err: err,
context: common.i18n.t('errors.services.mega.requestFailed.error')
}));
error = err.message;
}
const successes = meta.filter(response => (response instanceof bulkEmailService.SuccessfulBatch));
const failures = meta.filter(response => (response instanceof bulkEmailService.FailedBatch));
const batchStatus = successes.length ? 'submitted' : 'failed';
if (!error && failures.length) {
error = failures[0].error.message;
}
if (error && error.length > 2000) {
error = error.substring(0, 2000);
}
try {
// CASE: the batch partially succeeded
await models.Email.edit({
status: batchStatus,
meta: JSON.stringify(successes),
error: error,
error_data: JSON.stringify(failures) // NOTE:need to discuss how we store this
}, {
id: emailModel.id
});
} catch (err) {
common.logging.error(err);
}
}
function listen() {

5
core/server/translations/en.json
View File

@ -519,6 +519,11 @@
},
"loader": "Error trying to load YAML setting for {setting} from '{path}'.",
"ensureSettings": "Error trying to access settings files in {path}."
},
"mega": {
"requestFailed": {
"error" : "The email service was unable to send an email batch."
}
}
},
"errors": {