From a0e9be548d17a26077c4af09f5f9813a4da4b50f Mon Sep 17 00:00:00 2001 From: Dmitrii Maganov Date: Thu, 4 Jul 2019 23:05:23 +0300 Subject: [PATCH] Fix return duplicate transaction promise for standalone transactions (#3328) --- src/transaction.js | 15 +++++++++------ src/util/make-knex.js | 2 +- test/integration/builder/transaction.js | 7 +++---- test/unit/knex.js | 10 ++++++---- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/transaction.js b/src/transaction.js index 792e0771..0b26d527 100644 --- a/src/transaction.js +++ b/src/transaction.js @@ -8,7 +8,7 @@ const makeKnex = require('./util/make-knex'); const debug = Debug('knex:tx'); -const { uniqueId, isUndefined, get } = require('lodash'); +const { uniqueId, isUndefined } = require('lodash'); // Acts as a facade for a Promise, keeping the internal state // and managing any child transactions. @@ -75,7 +75,13 @@ class Transaction extends EventEmitter { return makeTransactor(this, connection, trxClient); }) .then((transactor) => { - transactor.executionPromise = executionPromise; + if (this.initPromise) { + transactor.executionPromise = executionPromise.catch((err) => { + throw err; + }); + } else { + transactor.executionPromise = executionPromise; + } // If we've returned a "thenable" from the transaction container, assume // the rollback and commit are chained to this object's success / failure. @@ -177,10 +183,7 @@ class Transaction extends EventEmitter { } if (status === 2) { if (isUndefined(value)) { - if ( - get(res, 'context.sql', '').toUpperCase() === 'ROLLBACK' && - this.doNotRejectOnRollback - ) { + if (this.doNotRejectOnRollback && /^ROLLBACK\b/i.test(sql)) { this._resolver(); return; } diff --git a/src/util/make-knex.js b/src/util/make-knex.js index 1832cffe..18b33f34 100644 --- a/src/util/make-knex.js +++ b/src/util/make-knex.js @@ -53,7 +53,7 @@ function initContext(knexFn) { let trx; return () => { if (!trx) { - trx = this.transaction(config); + trx = this.transaction(undefined, config); } return trx; }; diff --git a/test/integration/builder/transaction.js b/test/integration/builder/transaction.js index 1def16d6..c9e70096 100644 --- a/test/integration/builder/transaction.js +++ b/test/integration/builder/transaction.js @@ -377,11 +377,10 @@ module.exports = function(knex) { it('does not reject promise when rolling back a transaction', async () => { const trxProvider = knex.transactionProvider(); - const trxPromise = trxProvider(); + const trx = await trxProvider(); - await trxPromise.then((trx) => { - return trx.rollback(); - }); + await trx.rollback(); + await trx.executionPromise; }); it('should allow for nested transactions', function() { diff --git a/test/unit/knex.js b/test/unit/knex.js index f1d15011..d4fe13f4 100644 --- a/test/unit/knex.js +++ b/test/unit/knex.js @@ -323,6 +323,9 @@ describe('knex', () => { .then((rows) => { expect(rows[0].result).to.equal(1); return transaction.commit(); + }) + .then(() => { + return transaction.executionPromise; }); }); @@ -405,11 +408,10 @@ describe('knex', () => { it('does not reject promise when rolling back a transaction', async () => { const knex = Knex(sqliteConfig); const trxProvider = knex.transactionProvider(); - const trxPromise = trxProvider(); + const trx = await trxProvider(); - await trxPromise.then((trx) => { - return trx.rollback(); - }); + await trx.rollback(); + await trx.executionPromise; }); it('creating transaction copy with user params should throw an error', () => {