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
Do not reject promise on transaction rollback #3235
Conversation
@elhigu I've updated implementation to have explicit checks that we actually were rollbacking ourselves. What do you think? |
…//github.com/tgriesser/knex into fix/do-not-reject-on-rollback # Conflicts: # src/transaction.js # test/unit/knex.js
Sounds like a good idea! I didn’t see any problems with code either. 👍 I should write out some tests which actually tests what happens in knex In different dialects when query throws e.g. conflict error inside transaction which is not leaked out of handler and not causing knex side rollback. Currently I know only more or less what pg dialect does in that case. |
Does that need to happen in this pr? |
no, thats why I said |
@elhigu Thought so, but clarified just in case, as you didn't approve PR either :-D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didnt find this approval form from phone yesterday 😅
Ahaha, yeah, mobile version is a mess. Thanks! |
fixes #3246