Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw an exception to rollback transaction if SQL migration fails #117

Merged
merged 6 commits into from
Mar 25, 2021

Conversation

cham11ng
Copy link
Contributor

Problem:

  • db.connection.transaction(async trx => { } didn't rollback the transaction because no exception was thrown in SQL migration process. In case of migration and rollback trx.migrate and trx.rollback transaction rollback is handled internally.

  • I've to remove the log from getMigrationName because it called more than a time which pollutes the console for a large number of migration files.

@cham11ng cham11ng added the bug Something isn't working label Mar 23, 2021
@cham11ng cham11ng self-assigned this Mar 23, 2021
@cham11ng cham11ng force-pushed the fix-transaction-issue branch from ddf98f0 to fb0c762 Compare March 23, 2021 14:25
@cham11ng cham11ng changed the title Throw an exception to rollback transaction if SQL migration fails Explicitly rollback transaction if SQL migration fails Mar 24, 2021
@cham11ng cham11ng force-pushed the fix-transaction-issue branch from 3acb763 to 01bd804 Compare March 24, 2021 10:44
@cham11ng cham11ng changed the title Explicitly rollback transaction if SQL migration fails [Do-Not-Merge] Explicitly rollback transaction if SQL migration fails Mar 24, 2021
@cham11ng cham11ng marked this pull request as draft March 24, 2021 11:55
@cham11ng cham11ng changed the title [Do-Not-Merge] Explicitly rollback transaction if SQL migration fails Throw an exception to rollback transaction if SQL migration fails Mar 24, 2021
@cham11ng cham11ng force-pushed the fix-transaction-issue branch from 01bd804 to 9f52d7b Compare March 24, 2021 12:48
@cham11ng cham11ng force-pushed the fix-transaction-issue branch from 1978596 to 15a8aae Compare March 25, 2021 04:55
@cham11ng cham11ng marked this pull request as ready for review March 25, 2021 04:55
@@ -25,7 +25,11 @@ export async function runSequentially<T>(promisers: Promiser<T>[]): Promise<T[]>

result.push(value);
} catch (err) {
throw err;
if (!err.result) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still gimmicky. Does the job for now though.

@mesaugat mesaugat merged commit fac47a2 into master Mar 25, 2021
@mesaugat mesaugat deleted the fix-transaction-issue branch March 25, 2021 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants