node-mysql transaction is not rolling back after unique key violation

Question

I'm testing node-mysql transactions in my application by performing an INSERT that will violate a UNIQUE key constraint, after first performing an UPDATE. While the INSERT fails, I find that the initial UPDATE is succeeding, despite the transaction with ROLLBACK's. A MySQL UPDATE doesn't perform an implicit commit, so I'm not sure what exactly is going on here.

pool.getConnection(function (err, connection) {

    connection.beginTransaction(function (err) {
        if (err) {
            console.log(" --- error 1: " + JSON.stringify(err));
        }

        console.log(" --- A");

        connection.query('update course_person set rescheduled_date = CURDATE() where idperson = ? and rescheduled_date is null', [idperson], function (err, rows, fields) {

            if (err) {
                connection.rollback(function () {
                    console.log(" --- error 2: " + JSON.stringify(err));
                });

                console.log(" --- B");

            }

            console.log(" --- C");

        });

        connection.query('insert into course_person (idcourse, idperson) VALUES (?, ?)', [newidcourse, idperson], function (err, rows, fields) {

            if (err) {
                connection.rollback(function () {
                    console.log(" --- error 3: " + JSON.stringify(err));
                });

                console.log(" --- D");

            }

            console.log(" --- E");

        });

        connection.commit(function (err) {
            if (err) {
                connection.rollback(function () {
                    console.log(" --- error 4: " + JSON.stringify(err));
                });

                console.log(" --- F");

            }

            console.log(" --- G");

            req.flash('success', 'Person was updated successfully.');
            res.redirect('/person/' + idperson);

            connection.release();
        });

    });

});

I'm getting the following output sequence:

 --- A
 --- C
 --- D
 --- E
 --- G
 --- error 3: {"code":"ER_DUP_ENTRY","errno":1062,"sqlState":"23000","index":0}

And Here's my proof :)

Answer 1

If you want asynchronous functions to run sequentially, you'll have to either nest them, or call them as part of an array of promises, or use async/await style code...something. What you're doing here is calling

1. pool.getConnection
2. connection.beginTransaction
3. connection.query & connection.query & connection.commit all at the same time

Off the top of my head, I'd try something like this:

router.get('/myfunc', async (req, res) => {

  try {

    const connection = await pool.getConnection();

    try {

      await connection.beginTransaction();

      // update statement
      const sql_update = `update course_person
                          set rescheduled_date = CURDATE()
                          where idperson = ?
                          and rescheduled_date is null`;
      await connection.query(sql_update, [idperson]);

      // insert statement
      const sql_insert = `insert into course_person (idcourse, idperson) VALUES (?, ?)`;
      await connection.query(sql_insert, [newidcourse, idperson]);

      // commit
      await connection.commit();

      req.flash('success', 'Person was updated successfully.');
      return res.redirect('/person/' + idperson);

    } finally {
      pool.releaseConnection(connection);       
    }

  } catch (err) {
    await connection.rollback();
    console.log(err);
  }

});

Note: I haven't tested this code. But this the pattern I use.

This will require that you use promise-mysql instead of node-mysql because async/await syntax needs promises to work with. You could also use Bluebird to promisify node-mysql. Also, if your version of node doesn't support async/await yet, you'll need to use something like babel to transpile it down. (I use babel in my workflow.)

If you wanted to simply nest the asynchronous calls instead, for a more traditional coding style you could do something like this:

pool.getConnection(function (err, connection) {

  connection.beginTransaction(function (err) {
    if (err) {
      console.log(" --- error 1: " +  JSON.stringify(err));
      throw err;
    }

    connection.query('update course_person set rescheduled_date = CURDATE() where idperson = ? and rescheduled_date is null', [idperson], function (err, rows, fields) {
      if (err) {
        connection.rollback(function () {
          console.log(" --- error 2: " + JSON.stringify(err));
          throw err;
        });
      }

      connection.query('insert into course_person (idcourse, idperson) VALUES (?, ?)', [newidcourse, idperson], function (err, rows, fields) {
        if (err) {
          connection.rollback(function () {
            console.log(" --- error 3: " + JSON.stringify(err));
            throw err;
          });
        }

        connection.commit(function (err) {
          if (err) {
            connection.rollback(function () {
              console.log(" --- error 4: " + JSON.stringify(err));
              throw err;
            });
          }

          req.flash('success', 'Person was updated successfully.');
          res.redirect('/person/' + idperson);

          connection.release();
        });
      });
    });
  });
});

Note how each call is nested inside the callback function of the previous call. This starts to get messy with all the indentation (i.e. "callback hell"). Code that uses promises or async/await is more readable and easier to maintain in the long run.