fix(transactions): rollback upon query error

README.md

@@ -383,6 +383,23 @@ let results = await mysql.transaction()
 
 If the record to `DELETE` doesn't exist, the `UPDATE` will not be performed. If the `UPDATE` fails, the `DELETE` will be rolled back.
 
+JavaScript errors thrown inside query functions will also trigger a rollback. For example:
+
+```javascript
+let results = await mysql.transaction()
+  .query('INSERT INTO table (x) VALUES(?)', [1])
+  .query(() => {
+    if (someCondition) {
+      throw new Error('Invalid condition'); // This will trigger a rollback
+    }
+    return ['UPDATE table SET x = 1'];
+  })
+  .rollback(e => { /* do something with the error */ }) // optional
+  .commit() // execute the queries
+```
+
+If `someCondition` is true, the error will be thrown, the transaction will be rolled back, and no queries will be executed.
+
 **NOTE:** Transaction support is designed for InnoDB tables (default). Other table types may not behave as expected.
 
 ## Reusing Persistent Connections

index.js

@@ -445,23 +445,61 @@ module.exports = (params) => {
   const commit = async (queries, rollback) => {
 
     let results = [] // keep track of results
+    let transactionStarted = false
 
-    // Start a transaction
-    await query('START TRANSACTION')
+    try {
+      // Start a transaction
+      await query('START TRANSACTION')
+      transactionStarted = true
+
+      // Loop through queries
+      for (let i = 0; i < queries.length; i++) {
+        try {
+          // Get the query arguments by calling the function
+          const queryArgs = queries[i](results[results.length - 1], results)
+
+          // If queryArgs is null or undefined, skip this query
+          if (queryArgs === null || queryArgs === undefined) {
+            continue
+          }
 
-    // Loop through queries
-    for (let i = 0; i < queries.length; i++) {
-      // Execute the queries, pass the rollback as context
-      let result = await query.apply({ rollback }, queries[i](results[results.length - 1], results))
-      // Add the result to the main results accumulator
-      results.push(result)
-    }
+          // Execute the queries, pass the rollback as context
+          let result = await query.apply({ rollback }, queryArgs)
+          // Add the result to the main results accumulator
+          results.push(result)
+        } catch (err) {
+          // If there's a JavaScript error in the query function, roll back and rethrow
+          if (transactionStarted) {
+            await query('ROLLBACK')
+          }
+          throw err
+        }
+      }
 
-    // Commit our transaction
-    await query('COMMIT')
+      // Commit our transaction
+      await query('COMMIT')
+
+      // Return the results
+      return results
+    } catch (err) {
+      // If there's an error during the transaction, roll back if needed
+      if (transactionStarted) {
+        try {
+          await query('ROLLBACK')
+        } catch (rollbackErr) {
+          // If rollback fails, log it but throw the original error
+          onError(rollbackErr)
+        }
+      }
 
-    // Return the results
-    return results
+      // Call the rollback handler if provided
+      if (typeof rollback === 'function') {
+        rollback(err)
+      }
+
+      // Rethrow the original error
+      throw err
+    }
   }
 
 

test/integration/transaction-js-error.spec.js

@@ -0,0 +1,81 @@
+const { expect } = require('chai');
+const {
+    createTestConnection,
+    setupTestTable,
+    cleanupTestTable,
+    closeConnection
+} = require('./helpers/setup');
+
+describe('Transaction JavaScript Error Tests', function () {
+    this.timeout(15000);
+
+    let db;
+    const TEST_TABLE = 'test_transaction_js_error';
+    const TABLE_SCHEMA = `
+        id INT AUTO_INCREMENT PRIMARY KEY,
+        created_at DATETIME,
+        description VARCHAR(255)
+    `;
+
+    before(async function () {
+        // Initialize the database connection
+        db = createTestConnection({ manageConns: false });
+        await setupTestTable(db, TEST_TABLE, TABLE_SCHEMA);
+    });
+
+    after(async function () {
+        try {
+            await cleanupTestTable(db, TEST_TABLE);
+        } finally {
+            await closeConnection(db);
+        }
+    });
+
+    beforeEach(async function () {
+        // Clear the table before each test
+        await db.query(`TRUNCATE TABLE ${TEST_TABLE}`);
+    });
+
+    it('should not execute stale queries when a JavaScript error is thrown in a transaction', async function () {
+        const firstDate = new Date(2023, 0, 1); // January 1, 2023
+
+        // First transaction attempt with a JavaScript error
+        try {
+            await db
+                .transaction()
+                .query(() => [`INSERT INTO ${TEST_TABLE} (created_at, description) VALUES(?, ?)`, [firstDate, 'First transaction']])
+                .query(() => {
+                    throw new Error('Abort transaction');
+                })
+                .commit();
+
+            // Should not reach here
+            expect.fail('Transaction should have failed');
+        } catch (error) {
+            expect(error.message).to.equal('Abort transaction');
+        }
+
+        // Check that no records were inserted from the failed transaction
+        const result1 = await db.query(`SELECT COUNT(*) as count FROM ${TEST_TABLE}`);
+        expect(result1[0].count).to.equal(0, 'No records should be inserted after the failed transaction');
+
+        // Second transaction - this one should succeed
+        const secondDate = new Date(2023, 1, 1); // February 1, 2023
+        await db
+            .transaction()
+            .query(() => [`INSERT INTO ${TEST_TABLE} (created_at, description) VALUES(?, ?)`, [secondDate, 'Second transaction']])
+            .commit();
+
+        // Check that only the second transaction's records were inserted
+        const result2 = await db.query(`SELECT * FROM ${TEST_TABLE} ORDER BY created_at`);
+        expect(result2.length).to.equal(1, 'Only one record should be inserted');
+        expect(result2[0].description).to.equal('Second transaction', 'Only the second transaction should be committed');
+
+        // The key test: Make sure the first transaction's query wasn't executed
+        const firstTransactionRecords = await db.query(
+            `SELECT COUNT(*) as count FROM ${TEST_TABLE} WHERE description = ?`,
+            ['First transaction']
+        );
+        expect(firstTransactionRecords[0].count).to.equal(0, 'First transaction should not have been executed');
+    });
+});