mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-25 11:55:03 +03:00
Added Sentry logging in BatchSendingService (#19321)
refs ARCH-25 - Added a log message if the email_count on an email differs from the totalCount calculated while creating batches by more than 1%, so we can investigate further.
This commit is contained in:
parent
565b9b245e
commit
24910a6ef2
@ -278,6 +278,14 @@ class BatchSendingService {
|
||||
if (email.get('email_count') !== totalCount) {
|
||||
logging.error(`Email ${email.id} has wrong stored email_count ${email.get('email_count')}, did expect ${totalCount}. Updating the model.`);
|
||||
|
||||
// If the error rate is greater than 1%, we log it to Sentry so we can investigate
|
||||
// Some differences are expected, e.g. if a new member signs up while we are sending the email
|
||||
const errorRate = Math.abs((totalCount - email.get('email_count')) / email.get('email_count'));
|
||||
if (this.#sentry && errorRate >= 0.01) {
|
||||
// we don't have a real exception, so just log a message to Sentry
|
||||
this.#sentry.captureMessage(`Email ${email.id} has wrong stored email_count ${email.get('email_count')}, did expect ${totalCount}.`);
|
||||
}
|
||||
|
||||
// We update the email model because this might happen in rare cases where the initial member count changed (e.g. deleted members)
|
||||
// between creating the email and sending it
|
||||
await email.save({
|
||||
|
@ -282,7 +282,7 @@ describe('Batch Sending Service', function () {
|
||||
]
|
||||
}));
|
||||
|
||||
const innitialMembers = members.slice();
|
||||
const initialMembers = members.slice();
|
||||
|
||||
Member.getFilteredCollectionQuery = ({filter}) => {
|
||||
// Everytime we request the members, we also create a new member, to simulate that creating batches doesn't happen in a transaction
|
||||
@ -361,12 +361,94 @@ describe('Batch Sending Service', function () {
|
||||
assert.equal(insertedRecipients.length, 16);
|
||||
|
||||
// Check all recipients match initialMembers
|
||||
assert.deepEqual(insertedRecipients.map(recipient => recipient.member_id).sort(), innitialMembers.map(member => member.id).sort());
|
||||
assert.deepEqual(insertedRecipients.map(recipient => recipient.member_id).sort(), initialMembers.map(member => member.id).sort());
|
||||
|
||||
// Check email_count set
|
||||
assert.equal(email.get('email_count'), 16);
|
||||
});
|
||||
|
||||
it('Does log message to sentry if email_count is off by > 1%', async function () {
|
||||
const Member = createModelClass({});
|
||||
const EmailBatch = createModelClass({});
|
||||
const newsletter = createModel({});
|
||||
|
||||
// Create 16 members in single line
|
||||
const members = new Array(16).fill(0).map(i => createModel({
|
||||
email: `example${i}@example.com`,
|
||||
uuid: `member${i}`,
|
||||
newsletters: [
|
||||
newsletter
|
||||
]
|
||||
}));
|
||||
|
||||
Member.getFilteredCollectionQuery = ({filter}) => {
|
||||
// Everytime we request the members, we also create a new member, to simulate that creating batches doesn't happen in a transaction
|
||||
// These created members should be excluded
|
||||
members.push(createModel({
|
||||
email: `example${members.length}@example.com`,
|
||||
uuid: `member${members.length}`,
|
||||
newsletters: [
|
||||
newsletter
|
||||
]
|
||||
}));
|
||||
|
||||
const q = nql(filter);
|
||||
// Check that the filter id:<${lastId} is a string
|
||||
// In rare cases when the object ID is numeric, the query returns unexpected results
|
||||
assert.equal(typeof q.toJSON().$and[1].id.$lt, 'string');
|
||||
|
||||
const all = members.filter((member) => {
|
||||
return q.queryJSON(member.toJSON());
|
||||
});
|
||||
|
||||
// Sort all by id desc (string)
|
||||
all.sort((a, b) => {
|
||||
return b.id.localeCompare(a.id);
|
||||
});
|
||||
return createDb({
|
||||
all: all.map(member => member.toJSON())
|
||||
});
|
||||
};
|
||||
|
||||
const db = createDb({});
|
||||
const captureMessage = sinon.stub();
|
||||
|
||||
const service = new BatchSendingService({
|
||||
models: {Member, EmailBatch},
|
||||
sentry: {
|
||||
captureMessage
|
||||
},
|
||||
emailRenderer: {
|
||||
getSegments() {
|
||||
return [null];
|
||||
}
|
||||
},
|
||||
sendingService: {
|
||||
getMaximumRecipients() {
|
||||
return 5;
|
||||
}
|
||||
},
|
||||
emailSegmenter: {
|
||||
getMemberFilterForSegment(n) {
|
||||
return `newsletters.id:'${n.id}'`;
|
||||
}
|
||||
},
|
||||
db
|
||||
});
|
||||
|
||||
const email = createModel({
|
||||
email_count: 15
|
||||
});
|
||||
|
||||
await service.createBatches({
|
||||
email,
|
||||
post: createModel({}),
|
||||
newsletter
|
||||
});
|
||||
|
||||
assert(captureMessage.calledOnce);
|
||||
});
|
||||
|
||||
it('works with multiple batches', async function () {
|
||||
const Member = createModelClass({});
|
||||
const EmailBatch = createModelClass({});
|
||||
@ -392,7 +474,7 @@ describe('Batch Sending Service', function () {
|
||||
}))
|
||||
];
|
||||
|
||||
const innitialMembers = members.slice();
|
||||
const initialMembers = members.slice();
|
||||
|
||||
Member.getFilteredCollectionQuery = ({filter}) => {
|
||||
const q = nql(filter);
|
||||
@ -452,7 +534,7 @@ describe('Batch Sending Service', function () {
|
||||
assert.equal(insertedRecipients.length, 4);
|
||||
|
||||
// Check all recipients match initialMembers
|
||||
assert.deepEqual(insertedRecipients.map(recipient => recipient.member_id).sort(), innitialMembers.map(member => member.id).sort());
|
||||
assert.deepEqual(insertedRecipients.map(recipient => recipient.member_id).sort(), initialMembers.map(member => member.id).sort());
|
||||
|
||||
// Check email_count set
|
||||
assert.equal(email.get('email_count'), 4);
|
||||
|
Loading…
Reference in New Issue
Block a user