0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-15 03:01:37 -05:00

🐛 Fixed member importer crash for failed imports (#15560)

refs/closes https://github.com/TryGhost/Team/issues/2004

- for imports, members are created inside a transaction, which causes the member created events to be dispatched.
- its possible that transactions for import can be rolled back if for some reason there is an error down the line while inserting other member properties. The rollback doesn't commit the member to DB, but the event dispatched earlier will still try to create the member created event which fails due to missing member id.
- knex transactions resolve the `executionPromise` both in case of explicit commit or rollback from the user, so just the transaction end check will not be good enough to make sure the member exists in DB
- adds explicit config to knex to reject transaction in case of rollback, which is then caught and event is not dispatched
This commit is contained in:
Rishabh Garg 2022-10-07 19:15:18 +05:30 committed by GitHub
parent c4188c1a9e
commit 8a598fe721
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 27 deletions

View file

@ -91,6 +91,19 @@ module.exports = class MemberRepository {
});
}
dispatchEvent(event, options) {
if (options?.transacting) {
// Only dispatch the event after the transaction has finished
options.transacting.executionPromise.then(async () => {
DomainEvents.dispatch(event);
}).catch(() => {
// catches transaction errors/rollback to not dispatch event
});
} else {
DomainEvents.dispatch(event);
}
}
isActiveSubscriptionStatus(status) {
return ['active', 'trialing', 'unpaid', 'past_due'].includes(status);
}
@ -304,10 +317,10 @@ module.exports = class MemberRepository {
}
if (newsletters && newsletters.length > 0) {
DomainEvents.dispatch(MemberSubscribeEvent.create({
this.dispatchEvent(MemberSubscribeEvent.create({
memberId: member.id,
source: source
}, eventData.created_at));
}, eventData.created_at), options);
}
// For paid members created via stripe checkout webhook event, link subscription
@ -337,12 +350,11 @@ module.exports = class MemberRepository {
}
}
}
DomainEvents.dispatch(MemberCreatedEvent.create({
this.dispatchEvent(MemberCreatedEvent.create({
memberId: member.id,
attribution: data.attribution,
source
}, eventData.created_at));
}, eventData.created_at), options);
return member;
}
@ -560,10 +572,10 @@ module.exports = class MemberRepository {
}
if (newslettersToAdd.length > 0 || newslettersToRemove.length > 0) {
DomainEvents.dispatch(MemberSubscribeEvent.create({
this.dispatchEvent(MemberSubscribeEvent.create({
memberId: member.id,
source: source
}, member.updated_at));
}, member.updated_at), sharedOptions);
}
if (member.attributes.email !== member._previousAttributes.email) {
@ -1003,16 +1015,7 @@ module.exports = class MemberRepository {
offerId: data.offerId,
attribution: data.attribution
});
if (options?.transacting) {
// Only dispatch the event after the transaction has finished
// Because else the offer won't be committed to the database yet
options.transacting.executionPromise.then(() => {
DomainEvents.dispatch(event);
});
} else {
DomainEvents.dispatch(event);
}
this.dispatchEvent(event, options);
}
let memberProducts = (await member.related('products').fetch(options)).toJSON();
@ -1306,15 +1309,7 @@ module.exports = class MemberRepository {
subscriptionId: subscriptionModel.get('id'),
tierId: ghostProduct?.get('id')
};
if (options?.transacting) {
// Only dispatch the event after the transaction has finished
// Because else the offer won't be committed to the database yet
options.transacting.executionPromise.then(() => {
DomainEvents.dispatch(SubscriptionCancelledEvent.create(cancelEventData, cancellationTimestamp));
});
} else {
DomainEvents.dispatch(SubscriptionCancelledEvent.create(cancelEventData, cancellationTimestamp));
}
this.dispatchEvent(SubscriptionCancelledEvent.create(cancelEventData, cancellationTimestamp), options);
}
}
}

View file

@ -134,7 +134,9 @@ module.exports = class MembersCSVImporter {
const result = await rows.reduce(async (resultPromise, row) => {
const resultAccumulator = await resultPromise;
const trx = await this._knex.transaction();
// Use doNotReject config to reject `executionPromise` on rollback
// https://github.com/knex/knex/blob/master/UPGRADING.md
const trx = await this._knex.transaction(undefined, {doNotRejectOnRollback: false});
const options = {
transacting: trx,
context: this._context