Failing test for savepoint does not exist error

[dplewis]

This is a first pass for solving an issue with savepoints being release by higher-level savepoints

This is my first functional PR. Let me know if anything is needed

TODO: Replace iit for it in test suite

Unhandled rejection BatchError: savepoint "sp_1_2" does not exist
    at check (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/spex/lib/ext/batch.js:135:32)
    at step (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/spex/lib/ext/batch.js:109:17)
    at utils.resolve.call.reason (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/spex/lib/ext/batch.js:83:17)
    at value.then.catch.error (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/spex/lib/utils/index.js:62:25)
    at tryCatcher (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/promise.js:725:18)
    at _drainQueueStep (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/Users/dplewis/Documents/GitHub/pg-promise/node_modules/bluebird/js/release/async.js:15:14)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

P.S. I get two test failing running on Mac OS 10.15.7

 1) Connection for invalid connection must report the right error
   Message:
     Expected 'database "bla-bla" does not exist' to contain 'password authentication failed for user'.
   Stacktrace:
     Error: Expected 'database "bla-bla" does not exist' to contain 'password authentication failed for user'.
    at jasmine.Spec.it (/Users/dplewis/Documents/GitHub/pg-promise/test/db.spec.js:263:43)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)

  2) Connection for invalid user name must report the right error
   Message:
     Expected 'role "somebody" does not exist' to contain 'password authentication failed for user "somebody"'.
   Stacktrace:
     Error: Expected 'role "somebody" does not exist' to contain 'password authentication failed for user "somebody"'.
    at jasmine.Spec.it (/Users/dplewis/Documents/GitHub/pg-promise/test/db.spec.js:291:43)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)

Finished in 8.733 seconds
511 tests, 1592 assertions, 2 failures, 0 skipped

[dplewis]

@vitaly-t The build passed even though the test failed. Check Travis

[vitaly-t]

@dplewis I took my time to analyze what is going on. I can now conclusively state that there is no issue within pg-promise.

Here's what actually has been going on...

This library does permit asynchronous transaction execution like the one you showed in your test. However, this code itself is logically invalid. If you think about how savepoints operate, once you release a savepoint, it doesn't just release itself, but also releases all savepoints that were created in between. And for this reason it is invalid executing parallel sub-transactions (savepoints), because you stop controlling the rollback path. But this is not pg-promise issue. This library cannot control such things, it cannot deduct it and somehow reroute savepoints release path, it is not possible. And from the point of PostgreSQL transaction logic, this is also not possible.

It is one such unique situation, when this library does permit you create such invalid code, while there is nothing it can do about it, and neither it should. It is strictly a matter of invalid transaction logic created by the client.

Up until changes that occurred within issue #774, savepoint names were less unique, and as a result, such invalid release was not detected, making the wrong assumption about a different savepoint release. This is why no error was thrown, even though the code logic did not execute correctly. After that change the savepoints logic became smarter, and it now detects when the client produces invalid savepoint logic structure. But this is as far as this library can go.

You have to synchronize such transactions, to avoid savepoints release clash:

await db.tx(async t => {
    const a = await t.tx(t1 => { });
    const b = await t.tx(t1 => { }); 
    const c = await t.tx(t1 => { });

    return {a, b, c}
});

[vitaly-t]

@dplewis

I therefore am closing this PR. Thank you for your help with this issue!!!

[dplewis]

@vitaly-t Thank you for looking into this issue. I learned a lot about how this amazing library works internally.

[vitaly-t]

@dplewis You are welcome! It was intersecting to myself also, to find an issue like that, and to see that it is in fact possible to generate invalid database logic within the existing architecture, even though the architecture here doesn't have any problem, it's just that JavaScript promises cannot be a 100% match to PostgreSQL transaction state machine, because the later does work a little different, wiping out inner savepoints when an outer is released. One just has to keep that in mind.

I should make time later on and document this somewhere, as this is a very interesting subject.