feat: add guards against starting parallel transactions using the sam…

…e connection

package.json

@@ -15,6 +15,7 @@
     "boolean": "^0.2.0",
     "camelcase": "^5.0.0",
     "crack-json": "^1.1.0",
+    "delay": "^4.1.0",
     "es6-error": "^4.1.1",
     "get-stack-trace": "^2.0.1",
     "pg": "^7.8.0",

src/binders/bindTransactionConnection.js

@@ -20,6 +20,7 @@ import type {
   DatabaseTransactionConnectionType,
   InternalDatabaseConnectionType,
   LoggerType,
+  TaggedTemplateLiteralInvocationType,
   TransactionFunctionType
 } from '../types';
 
@@ -29,16 +30,28 @@ export default (
   clientConfiguration: ClientConfigurationType,
   transactionDepth: number
 ): DatabaseTransactionConnectionType => {
+  const mapInvocation = (fn) => {
+    const bound = mapTaggedTemplateLiteralInvocation(fn);
+
+    return (taggedQuery: TaggedTemplateLiteralInvocationType) => {
+      if (transactionDepth !== connection.slonik.transactionDepth) {
+        return Promise.reject(new Error('Cannot run a query using parent transaction.'));
+      }
+
+      return bound(taggedQuery);
+    };
+  };
+
   return {
-    any: mapTaggedTemplateLiteralInvocation(any.bind(null, parentLog, connection, clientConfiguration)),
-    anyFirst: mapTaggedTemplateLiteralInvocation(anyFirst.bind(null, parentLog, connection, clientConfiguration)),
-    many: mapTaggedTemplateLiteralInvocation(many.bind(null, parentLog, connection, clientConfiguration)),
-    manyFirst: mapTaggedTemplateLiteralInvocation(manyFirst.bind(null, parentLog, connection, clientConfiguration)),
-    maybeOne: mapTaggedTemplateLiteralInvocation(maybeOne.bind(null, parentLog, connection, clientConfiguration)),
-    maybeOneFirst: mapTaggedTemplateLiteralInvocation(maybeOneFirst.bind(null, parentLog, connection, clientConfiguration)),
-    one: mapTaggedTemplateLiteralInvocation(one.bind(null, parentLog, connection, clientConfiguration)),
-    oneFirst: mapTaggedTemplateLiteralInvocation(oneFirst.bind(null, parentLog, connection, clientConfiguration)),
-    query: mapTaggedTemplateLiteralInvocation(query.bind(null, parentLog, connection, clientConfiguration)),
+    any: mapInvocation(any.bind(null, parentLog, connection, clientConfiguration)),
+    anyFirst: mapInvocation(anyFirst.bind(null, parentLog, connection, clientConfiguration)),
+    many: mapInvocation(many.bind(null, parentLog, connection, clientConfiguration)),
+    manyFirst: mapInvocation(manyFirst.bind(null, parentLog, connection, clientConfiguration)),
+    maybeOne: mapInvocation(maybeOne.bind(null, parentLog, connection, clientConfiguration)),
+    maybeOneFirst: mapInvocation(maybeOneFirst.bind(null, parentLog, connection, clientConfiguration)),
+    one: mapInvocation(one.bind(null, parentLog, connection, clientConfiguration)),
+    oneFirst: mapInvocation(oneFirst.bind(null, parentLog, connection, clientConfiguration)),
+    query: mapInvocation(query.bind(null, parentLog, connection, clientConfiguration)),
     transaction: (handler: TransactionFunctionType) => {
       return nestedTransaction(parentLog, connection, clientConfiguration, handler, transactionDepth);
     }

src/connectionMethods/nestedTransaction.js

@@ -21,6 +21,8 @@ const nestedTransaction: InternalNestedTransactionFunctionType = async (parentLo
   });
 
   try {
+    connection.slonik.transactionDepth = newTransactionDepth;
+
     const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, newTransactionDepth));
 
     return result;
@@ -32,6 +34,8 @@ const nestedTransaction: InternalNestedTransactionFunctionType = async (parentLo
     }, 'rolling back transaction due to an error');
 
     throw error;
+  } finally {
+    connection.slonik.transactionDepth = newTransactionDepth - 1;
   }
 };
 

src/connectionMethods/transaction.js

@@ -12,14 +12,20 @@ import type {
 } from '../types';
 
 const transaction: InternalTransactionFunctionType = async (parentLog, connection, clientConfiguration, handler) => {
+  if (connection.slonik.transactionDepth !== null) {
+    throw new Error('Cannot use the same connection to start a new transaction before completing the last transaction.');
+  }
+
+  connection.slonik.transactionDepth = 0;
+
   await connection.query('START TRANSACTION');
 
   const log = parentLog.child({
     transactionId: createUlid()
   });
 
   try {
-    const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, 0));
+    const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, connection.slonik.transactionDepth));
 
     await connection.query('COMMIT');
 
@@ -32,6 +38,8 @@ const transaction: InternalTransactionFunctionType = async (parentLog, connectio
     }, 'rolling back transaction due to an error');
 
     throw error;
+  } finally {
+    connection.slonik.transactionDepth = null;
   }
 };
 

test/slonik/binders/bindPool/transaction.js

@@ -2,6 +2,7 @@
 
 import test from 'ava';
 import sinon from 'sinon';
+import delay from 'delay';
 import sql from '../../../../src/templateTags/sql';
 import bindPool from '../../../../src/binders/bindPool';
 import log from '../../../helpers/Logger';
@@ -30,7 +31,10 @@ const createPool = () => {
     connect: () => {
       return connection;
     },
-    query: () => {}
+    query: () => {},
+    slonik: {
+      transactionDepth: null
+    }
   };
 
   const querySpy = sinon.spy(internalPool, 'query');
@@ -183,3 +187,34 @@ test('rollsback the entire transaction with multiple savepoints (multiple depth
     'ROLLBACK'
   ]);
 });
+
+test('throws an error if an attempt is made to create a new transaction before the last transaction is completed', async (t) => {
+  const pool = createPool();
+
+  const connection = pool.connect((c1) => {
+    return Promise.race([
+      c1.transaction(() => {
+        return delay(1000);
+      }),
+      c1.transaction(() => {
+        return delay(1000);
+      })
+    ]);
+  });
+
+  await t.throwsAsync(connection, 'Cannot use the same connection to start a new transaction before completing the last transaction.');
+});
+
+test('throws an error if an attempt is made to execute a query using the parent transaction before the current transaction is completed', async (t) => {
+  const pool = createPool();
+
+  const connection = pool.connect((c1) => {
+    return c1.transaction((t1) => {
+      return t1.transaction(() => {
+        return t1.query(sql`SELECT 1`);
+      });
+    });
+  });
+
+  await t.throwsAsync(connection, 'Cannot run a query using parent transaction.');
+});

test/slonik/connectionMethods/transaction.js

@@ -12,7 +12,10 @@ test('commits successful transaction', async (t) => {
   query.returns({});
 
   const connection = {
-    query
+    query,
+    slonik: {
+      transactionDepth: null
+    }
   };
 
   const result = await transaction(log, connection, createClientConfiguration(), async () => {
@@ -39,7 +42,10 @@ test('rollbacks unsuccessful transaction', async (t) => {
   query.returns({});
 
   const connection = {
-    query
+    query,
+    slonik: {
+      transactionDepth: null
+    }
   };
 
   const transactionExecution = transaction(log, connection, createClientConfiguration(), async () => {