DB transaction with callback

Question

I am trying to figure out how to correctly set this block of code up so that the commit function will wait until all of the rows are inserted. Currently I am reading a csv and need to insert a new row per column into a table. I also need to add a row into a parent table for that. I need all of that to finish before I call commit. I have yet to master callbacks so please be gentle.

db.beginTransaction(function (err) {
    if (err) 
    {
        //could not begin a transaction for some reason.
        logger.error("beginTransaction error: " +err);
    }
    //need to wrap this *************
    db.query("INSERT INTO TABLE VALUES ('" + unique_id + "','real_time','" + msg + "',1,1,LOCALTIMESTAMP)", function(err){
        if(err)
        {
            logger.error("error insert into parent table: "+err);
        }
    });

    for(var i = 0; i < headers.length; i++)
    {
        //replaces single quote (') with two single quotes to escape it ('')
        values[i] = values[i].replace("'","''");
        db.query("INSERT INTO TABLE VALUES ('" + unique_id + "','" + headers[i] + "',0,'" + values[i] + "')", function(err){
            if(err)
            {
                logger.error("error insert into child table: "+err);
            }
        });
    }
    //To here ************
    db.commitTransaction(function (err) {
        if (err) 
        {
            //error during commit
            logger.error("Commit error: "+err);
        }
    }); //end of commitTransaction
    callback();
});//End of beginTransaction

Answer 1

There's three basic ways of tackling this synchronization problem which I'll demonstrate here using new style arrow functions. The traditional Node way is with callbacks:

a((err, resultA) => {
  // Fires when A is done or errored out

  if (err) {
    // Log, panic, etc.
    return;
  }

  b((err, resultB) => {
    // Fires when A and B are done or A is done and B errored out

    if (err) {
      // Log, panic, etc.
      return;
    }

    c((err, resultC) => {
     // Fires when A, B and C are done or A and B are done and C errored out

      if (err) {
        // Log, panic, etc.
        return;
      }
    });
  });
});

This is what people refer to as "callback hell" because the nesting and error propagation code gets more and more ridiculous as your dependencies grow in complexity. I find this style unsustainable for any non-trivial application.

The next style is Promise-driven:

a().then(resultA => {
  // Fires when A is done
  return b();
}).then(resultB => {
  // Fires when B is done
  return c();
}).then(resultC => {
  // Fires when C is done
}).catch(err => {
  // Fires if any of the previous calls produce an error
});

This tends to be a lot "flatter" and easier to follow, but it's still a lot of heavy syntax for what should be simple. The newer async/await style builds on promises by adding support for them to the JavaScript syntax:

try {
  let resultA = await a();
  let resultB = await a();
  let resultC = await a();
} catch(err) {
  // Fires when any error occurs
}

This works inside any function tagged async like:

async function runQueries() {
  // async code
}

Which can make your life a lot easier. You can also use conventional try/catch notation for errors, they're propagated accordingly.

Answer 2

As tadman states, it's very important that you don't manually escape values and use parameterized queries instead. Make sure you fix this first.

Unfortunately it doesn't look like node-odbc supports promises. You may be able to get it to work with something like Bluebird.promisify. Currently what you want is to keep track of how many successful inserts were completed and then commit the transaction if they all complete successfully.

let successfulInsertions = 0;
let insertionAttempts = 0;
for(var i = 0; i < headers.length; i++) {
    db.query("INSERT INTO TABLE VALUES (?, ?, ?, ?)", params, err => {
        if (err) {
            logger.error("error insert into child table: "+err);
        }
        else {
          successfulInsertions++;
        }
        insertionAttempts++;

        if (insertionAttempts === headers.length) {
            if (successfulInsertions === insertionAttempts) {
                db.commitTransaction();
            }
            else {
                db.rollbackTransaction();
            }
            callback();
        }
    });
}

There are libraries out there that will help with this but they would require rewriting your code a bit. If you can use Bluebird's promisifyAll on the node-odbc library I would rewrite it using async/await (this is still asynchronous and functions the same):

await db.beginTransactionAsync();
try {
  await db.queryAsync("INSERT INTO TABLE VALUES (?, ?, ?, ?)", params);

  await Promise.all(headers.map((header, i) =>
    db.queryAsync("INSERT INTO TABLE VALUES (?, ?, ?, ?)", [unique_id, header, 0, values[i])
  );
  await db.commitTransactionAsync();
} catch (err) {
  logger.error(err);
  db.rollbackTransaction();
}

Note that if beginTransaction throws an error for some reason this code will throw an error.

Answer 3

I don't really understand your code, why don't you return or stop your code if there is an error?

Here for example, you should have your code like this

db.beginTransaction(function (err) {
    if (err) 
        return logger.error("beginTransaction error: " +err), db.rollback(/*...*/)
    //....
}

Second, you should change your whole code with the async/await syntax.

async function foo () {
    await new Promise((next, err)=> {
        db.beginTransaction(e => e ? err(e) : next())
    })

    await new Promise((next, err)=> {
        db.query(`INSERT INTO TABLE VALUES ('${unique_id}','real_time','${msg}',1,1,LOCALTIMESTAMP)`, e => e ? err(e) : next())
    })

    for (var i = 0; i < headers.length; i++) {
        await new Promise((next, err)=> {
            db.query(`INSERT INTO TABLE VALUES ('${unique_id}','${headers[i]}',0,'${values[i]}')`, e => e ? err(e) : next())
        })
    }

    await new Promise((next, err)=> {
        db.commitTransaction(e => e ? err(e) : next())
    })
}

foo()
.then(()=> callback())
.catch(e=> logger.error(`Error: ${e}`))