20

I am getting the correct output, and indeed, these two operations are being treated as a single transactional unit; where if one fails, both fail.

In this code example: i am doing a transaction of

(1) insert (2) update

The way I approach it is to nest my db operations inside the .then. My question is if this code is correct by accident? i am new to promises and knex.

knex.transaction(function(t) {
   knex('foo')
   .transacting(t)
   .insert({id:"asdfk", username:"barry", email:"barry@bar.com"})
   .then(function() {
       knex('foo')
       .where('username','=','bob')
       .update({email:"bob@foo.com"})
       .then(t.commit, t.rollback)
   })
})
.then(function() {
 // it worked
},
function() {
 // it failed
});

This works, but I feel like I am doing something wrong still. Looking for comments.

Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
david
  • 381
  • 1
  • 3
  • 7
  • Can you try 1) adding some console.logs where the `// it worked` and `// it failed` comments are, and 2) forcing the insert statement to fail somehow? With your current nesting, the t.rollback is only called when the update fails, so I'd imagine it wouldn't do the right thing if the insert fails. – user3374348 Apr 09 '14 at 11:39

2 Answers2

39

You need to return a promise from the inner query in order for the outer chain to be chained with that.

You also swallow any errors because you don't rethrow them - it's better to use .catch() for this reason because it makes it more clearer what is happening - that is what would happen with normal try-catch statement.

knex.transaction(function(t) {
   return knex('foo')
   .transacting(t)
   .insert({id:"asdfk", username:"barry", email:"barry@bar.com"})
   .then(function() {
        return knex('foo')
           .where('username','=','bob')
           .update({email:"bob@foo.com"});
   })
   .then(t.commit)
   .catch(function(e) {
        t.rollback();
        throw e;
   })
})
.then(function() {
 // it worked
})
.catch(function(e) {
 // it failed
});

To understand it better, here's the synchronous version that is being "emulated":

try {
    var t = knex.transaction();
    try {
        knex("foo")
            .transacting(t)
            .insert({id:"asdfk", username:"barry", email:"barry@bar.com"});
        knex("foo")
            .where('username','=','bob')
            .update({email:"bob@foo.com"});
        t.commit();
    }
    catch (e) {
        t.rollback();
        // As you can see, if you don't rethrow here
        // the outer catch is never triggered
        throw e;
    }
    // It worked
}
catch (e) {
    //It failed
}
Esailija
  • 138,174
  • 23
  • 272
  • 326
  • 5
    Really helpful, but didn't you missed a transacting on the update of the 2nd example? – John Dec 27 '17 at 14:22
  • 2
    In the second example, if you were to use a `SELECT` query in the transaction, how would you use it with `async`/`await`? – asafc Feb 02 '19 at 12:39
  • @Juan I am also wondering whether the transacting is needed for the update call or not. It should be there right? – Nirmal Gamage Jul 28 '21 at 08:52
  • I did use it multiple times whenever I needed, here you have a bunch of examples as well https://newbedev.com/get-knex-js-transactions-working-with-es7-async-await – John Jul 28 '21 at 15:55
5

I was trying out the accepted answer here. It was throwing me some errors like "Transaction query is already complete" and "Database locked". The answer is old, so might be working with previous version. I am using Sqlite3.34.1 and knex0.95.4. The code worked for me with some tweaks. Adding in this thread, It could help someone.

async function process() {
    await knex.transaction(function(t) {
        return knex('foo')
        .transacting(t)
        .insert({id:"asdfkg", username:"bob", email:"bob@bar.com"})
        .then(function() {
            return t('foo').insert({id:"abcd", username:"john", email:"john@bar.com"})
        })
        .then(function() {
            return t('foo')
            .where('username','=','bob')
            .update({email:"bob@foo.com"});
        })
    })
    .then(function() {
    console.log("it worked")
    })
    .catch(function(e) {
    console.log(e)
    console.log("It failed")
    });
    knex.destroy()
}

I think, rollback and commit is taken care by its own, we wont have to specify it explicitly.

Gunjan
  • 2,775
  • 27
  • 30
  • yes i think you are right. the documentation does mention that if returning a promise from the transaction callback function there is no need to explicitly call commit or rollback http://knexjs.org/guide/transactions.html – anushka Jul 21 '22 at 06:23