fix unhandledrejection for rollback of non managed transaction #3665

maximelkin · 2020-02-14T19:39:59Z

No description provided.

maximelkin · 2020-02-14T19:55:28Z

Somehow connected with switches between bluebird <-> native promises

briandamaged · 2020-02-14T20:00:07Z

Agreed, and I suspect that Promise.race(..) might be involved as well. The mysql / mysql2 dialects are repeatedly failing for me for this specific test case:

knex/test/integration/builder/additional.js

Line 863 in 77df705

    
           it('.timeout(ms, {cancel: true}) should throw error if cancellation cannot acquire connection', function() {

Both of these are implemented in terms of Promise.race(..). Additionally, it appears that there is a race between:

A standard node.js setTimeout(..) call
A resource acquisition within the Tarn library. (Specifically, when attempting to obtain a connection from a pool. The acquisition is timing out, but the promise rejection never ends up being handled)

briandamaged · 2020-02-14T20:10:56Z

lib/transaction.js

@@ -195,7 +195,7 @@ class Transaction extends EventEmitter {

            value = new Error(`Transaction rejected with non-error: ${value}`);
          }
-          this._rejecter(value);
+          setImmediate(() => this._rejecter(value));


I just tried out your fix, but I'm still observing the problem within this test case:

knex/test/integration/builder/additional.js

Line 863 in 77df705

it('.timeout(ms, {cancel: true}) should throw error if cancellation cannot acquire connection', function() {

Here's a sample of my output:

% DB="mysql mysql2" KNEX_TEST_TIMEOUT=60000 ./node_modules/.bin/mocha --exit -t 10000 test/index.js sqlite does not support inserting default values. Set the `useNullAsDefault` flag to hide this warning. (see docs http://knexjs.org/#Builder-insert). sqlite does not support inserting default values. Set the `useNullAsDefault` flag to hide this warning. (see docs http://knexjs.org/#Builder-insert). sqlite does not support inserting default values. Set the `useNullAsDefault` flag to hide this warning. (see docs http://knexjs.org/#Builder-insert). Integration Tests mysql | mysql Additional Unhandled rejection: KnexTimeoutError: Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call? at /Users/brianlauber/Documents/src/knex/lib/client.js:351:30 From previous event: at Client_MySQL.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/client.js:348:15) at Client_MySQL.cancelQuery (/Users/brianlauber/Documents/src/knex/lib/dialects/mysql/index.js:175:32) at /Users/brianlauber/Documents/src/knex/lib/runner.js:177:37 From previous event: at Runner.query (/Users/brianlauber/Documents/src/knex/lib/runner.js:169:13) at /Users/brianlauber/Documents/src/knex/lib/runner.js:41:23 at /Users/brianlauber/Documents/src/knex/lib/runner.js:261:24 From previous event: at Runner.ensureConnection (/Users/brianlauber/Documents/src/knex/lib/runner.js:259:8) at Runner.run (/Users/brianlauber/Documents/src/knex/lib/runner.js:27:12) at Raw.Target.then (/Users/brianlauber/Documents/src/knex/lib/interface.js:15:43) at Context.<anonymous> (/Users/brianlauber/Documents/src/knex/test/integration/builder/additional.js:908:10) at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21) at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7) at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14) at Immediate.<anonymous> (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5) at processImmediate (internal/timers.js:439:21) { name: 'KnexTimeoutError' } ✓ .timeout(ms, {cancel: true}) should throw error if cancellation cannot acquire connection (5135ms) mysql | mysql2 Additional Unhandled rejection: KnexTimeoutError: Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call? at /Users/brianlauber/Documents/src/knex/lib/client.js:351:30 From previous event: at Client_MySQL2.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/client.js:348:15) at Client_MySQL2.cancelQuery (/Users/brianlauber/Documents/src/knex/lib/dialects/mysql/index.js:175:32) at /Users/brianlauber/Documents/src/knex/lib/runner.js:177:37 From previous event: at Runner.query (/Users/brianlauber/Documents/src/knex/lib/runner.js:169:13) at /Users/brianlauber/Documents/src/knex/lib/runner.js:41:23 at /Users/brianlauber/Documents/src/knex/lib/runner.js:261:24 From previous event: at Runner.ensureConnection (/Users/brianlauber/Documents/src/knex/lib/runner.js:259:8) at Runner.run (/Users/brianlauber/Documents/src/knex/lib/runner.js:27:12) at Raw.Target.then (/Users/brianlauber/Documents/src/knex/lib/interface.js:15:43) at Context.<anonymous> (/Users/brianlauber/Documents/src/knex/test/integration/builder/additional.js:908:10) at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21) at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7) at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14) at Immediate.<anonymous> (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5) at processImmediate (internal/timers.js:439:21) { name: 'KnexTimeoutError' } ✓ .timeout(ms, {cancel: true}) should throw error if cancellation cannot acquire connection (5125ms) 2 passing (10s) Unhandled rejections: 2 { reason: KnexTimeoutError: Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call? at /Users/brianlauber/Documents/src/knex/lib/client.js:351:30 From previous event: at Client_MySQL.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/client.js:348:15) at Client_MySQL.cancelQuery (/Users/brianlauber/Documents/src/knex/lib/dialects/mysql/index.js:175:32) at /Users/brianlauber/Documents/src/knex/lib/runner.js:177:37 From previous event: at Runner.query (/Users/brianlauber/Documents/src/knex/lib/runner.js:169:13) at /Users/brianlauber/Documents/src/knex/lib/runner.js:41:23 at /Users/brianlauber/Documents/src/knex/lib/runner.js:261:24 From previous event: at Runner.ensureConnection (/Users/brianlauber/Documents/src/knex/lib/runner.js:259:8) at Runner.run (/Users/brianlauber/Documents/src/knex/lib/runner.js:27:12) at Raw.Target.then (/Users/brianlauber/Documents/src/knex/lib/interface.js:15:43) at Context.<anonymous> (/Users/brianlauber/Documents/src/knex/test/integration/builder/additional.js:908:10) at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21) at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7) at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14) at Immediate.<anonymous> (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5) at processImmediate (internal/timers.js:439:21) { name: 'KnexTimeoutError' } } { reason: KnexTimeoutError: Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call? at /Users/brianlauber/Documents/src/knex/lib/client.js:351:30 From previous event: at Client_MySQL2.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/client.js:348:15) at Client_MySQL2.cancelQuery (/Users/brianlauber/Documents/src/knex/lib/dialects/mysql/index.js:175:32) at /Users/brianlauber/Documents/src/knex/lib/runner.js:177:37 From previous event: at Runner.query (/Users/brianlauber/Documents/src/knex/lib/runner.js:169:13) at /Users/brianlauber/Documents/src/knex/lib/runner.js:41:23 at /Users/brianlauber/Documents/src/knex/lib/runner.js:261:24 From previous event: at Runner.ensureConnection (/Users/brianlauber/Documents/src/knex/lib/runner.js:259:8) at Runner.run (/Users/brianlauber/Documents/src/knex/lib/runner.js:27:12) at Raw.Target.then (/Users/brianlauber/Documents/src/knex/lib/interface.js:15:43) at Context.<anonymous> (/Users/brianlauber/Documents/src/knex/test/integration/builder/additional.js:908:10) at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21) at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7) at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14) at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7 at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14) at Immediate.<anonymous> (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5) at processImmediate (internal/timers.js:439:21) { name: 'KnexTimeoutError' } }

Q: what problem is this line intended to fix?

Unhandled rejection Error: Transaction rejected with non-error: undefined
from https://travis-ci.org/knex/knex/jobs/650139383?utm_medium=notification&utm_source=github_status

All troubles are going from

const trx = await knex.transaction(); trx.select() // ...blablabla... await trx.executionPromise

Gotcha. Have you been able to recreate this error after #3666 was merged? (The link you provided looks like it's from a Travis-CI run that is 5 days old)

k -- just got back, so I'll try recreating the issue in the next few minutes.

One other thing I noticed: it appears that some test cases will "fail" if you omit a specific database from the DB="..." environment variable. For instance: you will observe "failures" if you run the tests via:

DB="mysql mysql2" KNEX_TEST_TIMEOUT=60000 ./node_modules/.bin/mocha --exit -t 10000 test/index.js

However, these are not actually failures in the Knex library itself. It's actually a problem w/ the way the test cases are written. More specifically: the test cases are referencing data that is only available when "sqlite3" is specified within DB="..." . (BTW: I'm going to create a separate issue for this)

So, with this in mind: what command are you running to kick off the tests? It might provide some insight into why the tests are failing.

@kibertoad can you help with explanation, how .executionPromise should work?
executionPromise must be awaited immediately, or after transaction commit/rollback?

@briandamaged

mocha ./test/unit/knex.js --grep "^knex rejects execution promise if there was a manual rollback and transaction is set to reject$"

@maximelkin executionPromise is a way to expose state of transaction to the user, so that he can e. g. handle the rollback or proceed with the rest of the flow after commit has happened. Having that in mind, I'd say that it should be awaited after transaction commit/rollback.
Maybe we need to rename the field to make it more explicit, e. g. transactionPromise.

@maximelkin : I've been able to recreate the failure locally using the command-line you provided. I've identified a few suspicious sections of code, but so far none of them seem to directly impact this specific issue.

For example: take a look at the following:

knex/lib/transaction.js

Lines 228 to 233 in 7288608

return (this._previousSibling

? this._previousSibling.catch(() => {})

: Promise.resolve()

).then(function() {

return connection;

});

These lines seem strange for at least 2 reasons:

There is a .catch(()=> {}) statement, which might be hiding other bugs. (More specifically: it might be causing Node to overlook some "unresolved promises". This will require some more investigation to confirm this)

The conditional logic seems to be largely unnecessary. In particular, this._previousSibling should always be defined thanks to the following lines in the constructor:

knex/lib/transaction.js

Lines 124 to 128 in 7288608

this._previousSibling = Bluebird.resolve(true);

if (outerTx) {

if (outerTx._lastChild) this._previousSibling = outerTx._lastChild;

outerTx._lastChild = this._promise;

}

But yeah: so far, these oddities don't seem to have any direct impact on the issue you're observing.

cc: @kibertoad

maximelkin · 2020-02-14T20:29:43Z

$ node
async function main() {
const a = Promise.reject(new Error('A!'))
Promise.resolve(a.then(console.log)).catch(e => console.log('handled1'))
a.then(() => console.log('never'))
}

handled1
(node:7931) UnhandledPromiseRejectionWarning: Error: A!
at main (repl:2:26)
at repl:1:1
at ContextifyScript.Script.runInThisContext (vm.js:50:33)
at REPLServer.defaultEval (repl.js:240:29)
at bound (domain.js:301:14)
at REPLServer.runBound [as eval] (domain.js:314:12)
at REPLServer.onLine (repl.js:468:10)
at emitOne (events.js:121:20)
at REPLServer.emit (events.js:211:7)
at REPLServer.Interface._onLine (readline.js:280:10)
(node:7931) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:7931) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

this is case of 2 unhandled rejections

lib/dialects/mysql/index.js

kibertoad · 2020-02-16T19:07:09Z

@maximelkin Is this still useful with #3666 merged?

maximelkin · 2020-02-17T09:33:24Z

@kibertoad partially - need to revert lib/dialects/mysql/index.js

maximelkin · 2020-02-17T09:51:41Z

Third unhandled rejection is disappear, I will re-read code again later, because don't understand why
Upd: still exists

briandamaged · 2020-02-18T21:06:42Z

lib/transaction.js

+          // todo^ is this true?
+          transactor.executionPromise = transactor.executionPromise.catch((err) => {
+            throw err;
+          });


Hmm... I suspect that this isn't actually addressing the underlying race condition. Instead, it is just adding a slight delay in the event processing queue, which makes the race condition less likely to occur.

Yes, this change is wrong too

briandamaged · 2020-02-18T23:14:16Z

@maximelkin + @kibertoad : Here's a new piece of info that might help w/ the investigation.

First, here is the stacktrace that is attached to the "unresolved promise":

Error: Transaction rejected with non-error: undefined
    at q.trxClient.query.catch.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:196:21)
From previous event:
    at Transaction._promise.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:67:32)
    at Bluebird.then.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:236:24)
From previous event:
    at Transaction.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:234:8)
    at new Transaction (/Users/brianlauber/Documents/src/knex/lib/transaction.js:58:26)
    at Client_SQLite3.transaction (/Users/brianlauber/Documents/src/knex/lib/client.js:125:12)
    at Function.transaction (/Users/brianlauber/Documents/src/knex/lib/util/make-knex.js:40:31)
    at Context.it.only (/Users/brianlauber/Documents/src/knex/test/unit/knex.js:403:28)
    at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21)
    at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7)
    at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14)
    at Immediate._onImmediate (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)

I modified the test case so that it prints out the stacktrace for the exception that it catches. Interestingly, the value that gets printed changes depending upon whether or not the "unresolved promise" occurred.

When the "unresolved promise" occurs, the test case prints the following stacktrace:

Error: Transaction rejected with non-error: undefined
    at q.trxClient.query.catch.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:196:21)
From previous event:
    at Transaction._promise.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:67:32)
    at Bluebird.then.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:236:24)
From previous event:
    at Transaction.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:234:8)
    at new Transaction (/Users/brianlauber/Documents/src/knex/lib/transaction.js:58:26)
    at Client_SQLite3.transaction (/Users/brianlauber/Documents/src/knex/lib/client.js:125:12)
    at Function.transaction (/Users/brianlauber/Documents/src/knex/lib/util/make-knex.js:40:31)
    at Context.it.only (/Users/brianlauber/Documents/src/knex/test/unit/knex.js:403:28)
    at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21)
    at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7)
    at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14)
    at Immediate._onImmediate (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)
Error: Transaction rejected with non-error: undefined
    at q.trxClient.query.catch.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:196:21)
From previous event:
    at Transaction._promise.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:67:32)
    at Bluebird.then.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:236:24)
From previous event:
    at Transaction.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:234:8)
    at new Transaction (/Users/brianlauber/Documents/src/knex/lib/transaction.js:58:26)
    at Client_SQLite3.transaction (/Users/brianlauber/Documents/src/knex/lib/client.js:125:12)
    at Function.transaction (/Users/brianlauber/Documents/src/knex/lib/util/make-knex.js:40:31)
    at Context.it.only (/Users/brianlauber/Documents/src/knex/test/unit/knex.js:403:28)
    at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21)
    at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7)
    at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14)
    at Immediate._onImmediate (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)

Notice that this appears to contain the "unresolved promise" stacktrace as a subcomponent.

In contrast, here is what gets printed when the "unresolved promise" does NOT occur:

Error: Transaction rejected with non-error: undefined
    at q.trxClient.query.catch.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:196:21)
From previous event:
    at Transaction._promise.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:67:32)
    at Bluebird.then.then (/Users/brianlauber/Documents/src/knex/lib/transaction.js:236:24)
From previous event:
    at Transaction.acquireConnection (/Users/brianlauber/Documents/src/knex/lib/transaction.js:234:8)
    at new Transaction (/Users/brianlauber/Documents/src/knex/lib/transaction.js:58:26)
    at Client_SQLite3.transaction (/Users/brianlauber/Documents/src/knex/lib/client.js:125:12)
    at Function.transaction (/Users/brianlauber/Documents/src/knex/lib/util/make-knex.js:40:31)
    at Context.it.only (/Users/brianlauber/Documents/src/knex/test/unit/knex.js:403:28)
    at callFn (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:395:21)
    at Test.Runnable.run (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runnable.js:382:7)
    at Runner.runTest (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:541:10)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:667:12
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:450:14)
    at /Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:460:7
    at next (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:362:14)
    at Immediate._onImmediate (/Users/brianlauber/Documents/src/knex/node_modules/mocha/lib/runner.js:428:5)
    at runCallback (timers.js:810:20)
    at tryOnImmediate (timers.js:768:5)
    at processImmediate [as _immediateCallback] (timers.js:745:5)

As you can see, it matches the stacktrace that would have been included in the "unresolved promise".

So: I don't have any specific conclusions related to this yet. But, I suspect that this will help pinpoint the source of the race condition.

briandamaged · 2020-02-19T00:12:53Z

@maximelkin + @kibertoad : Update on my "different stacktraces" message (#3665 (comment))

Looks like that might have been a false alarm. It was a quirk in the console output that caused it to look like a really long stacktrace. In reality, the error was actually getting printed from 2 different places. (Which is why there the weird disconnect in the middle of the stacktrace)

briandamaged · 2020-02-19T01:40:16Z

@maximelkin + @kibertoad : FYI -- I created another PR for you to look at over here:

#3671

It was initially intended as a minor code cleanup; however, it might have inadvertently fixed the race condition as well.

maximelkin changed the title ~~bypass extra microtasks for non-managed transactions rollback call~~ fix unhandledrejection for rollback of non managed transaction Feb 14, 2020

maximelkin mentioned this pull request Feb 14, 2020

Fix: Knex CLI calls process.chdir() before opening Knexfile #3661

Merged

briandamaged reviewed Feb 14, 2020

View reviewed changes

lib/dialects/mysql/index.js Outdated Show resolved Hide resolved

briandamaged mentioned this pull request Feb 15, 2020

Fixed unresolved promise in cancelQuery(..) ... #3666

Merged

maximelkin force-pushed the fix_unhandled branch from 70edb3b to 8be47f4 Compare February 18, 2020 19:42

bypass extra microtasks for non-managed transactions rollback call

ba69919

maximelkin force-pushed the fix_unhandled branch from 8be47f4 to ba69919 Compare February 18, 2020 19:44

better fix

c8d3f7a

briandamaged reviewed Feb 18, 2020

View reviewed changes

briandamaged mentioned this pull request Feb 19, 2020

Fix race condition in non-container transactions #3671

Merged

maximelkin closed this Feb 20, 2020

maximelkin deleted the fix_unhandled branch February 20, 2020 10:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix unhandledrejection for rollback of non managed transaction #3665

fix unhandledrejection for rollback of non managed transaction #3665

maximelkin commented Feb 14, 2020

maximelkin commented Feb 14, 2020

briandamaged commented Feb 14, 2020

briandamaged Feb 14, 2020

briandamaged Feb 18, 2020

maximelkin Feb 18, 2020

maximelkin Feb 18, 2020

briandamaged Feb 18, 2020

briandamaged Feb 18, 2020

maximelkin Feb 18, 2020

maximelkin Feb 18, 2020

kibertoad Feb 18, 2020

briandamaged Feb 18, 2020

maximelkin commented Feb 14, 2020

kibertoad commented Feb 16, 2020

maximelkin commented Feb 17, 2020

maximelkin commented Feb 17, 2020 •

edited

briandamaged Feb 18, 2020

maximelkin Feb 18, 2020

briandamaged commented Feb 18, 2020 •

edited

briandamaged commented Feb 19, 2020

briandamaged commented Feb 19, 2020

	return (this._previousSibling
	? this._previousSibling.catch(() => {})
	: Promise.resolve()
	).then(function() {
	return connection;
	});

	this._previousSibling = Bluebird.resolve(true);
	if (outerTx) {
	if (outerTx._lastChild) this._previousSibling = outerTx._lastChild;
	outerTx._lastChild = this._promise;
	}

fix unhandledrejection for rollback of non managed transaction #3665

fix unhandledrejection for rollback of non managed transaction #3665

Conversation

maximelkin commented Feb 14, 2020

maximelkin commented Feb 14, 2020

briandamaged commented Feb 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximelkin commented Feb 14, 2020

kibertoad commented Feb 16, 2020

maximelkin commented Feb 17, 2020

maximelkin commented Feb 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

briandamaged commented Feb 18, 2020 • edited

briandamaged commented Feb 19, 2020

briandamaged commented Feb 19, 2020

maximelkin commented Feb 17, 2020 •

edited

briandamaged commented Feb 18, 2020 •

edited