Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

knex transaction with async/await #1764

Closed
ghost opened this issue Oct 30, 2016 · 9 comments
Closed

knex transaction with async/await #1764

ghost opened this issue Oct 30, 2016 · 9 comments

Comments

@ghost
Copy link

ghost commented Oct 30, 2016

Hey guys,

Im trying to use a transaction. The statements works and takes effect. But if an error comes, the effect still there and there is no rollback.

Here is the code where Im testing my transactions.

    knex.transaction(async function (trx) {

        try {
            let objs = await trx.select('id', 'money').from('account')
            const accOne = objs[0]
            const accTwo = objs[1]

            accOne.money -= 1
            accTwo.money += 1
            await trx.where('id', accOne.id).update(accOne).into('account')
            await trx.where('id', accTwo.id).update(accTwo).into('account')

        } catch (err) {
            await console.log(err)
        }
    })
@elhigu
Copy link
Member

elhigu commented Nov 1, 2016

you are consuming your exception inside the aync transaction callback so naturally it seems that transaction was ran ok and will be committed.

ps. you don't have to await sync functions like console.log

@elhigu elhigu closed this as completed Nov 1, 2016
@ghost
Copy link
Author

ghost commented Nov 1, 2016

Yeah i know with console.log. I dont know why i wrote it :D Maybe copy paste error.

So, what exactly is wrong? The Transactions are correctly, but if there is an error, then its not rollback.

@elhigu
Copy link
Member

elhigu commented Nov 1, 2016

you are consuming your exception inside the async transaction handler

    knex.transaction(async function (trx) {

        try {
            let objs = await trx.select('id', 'money').from('account')
            const accOne = objs[0]
            const accTwo = objs[1]

            accOne.money -= 1
            accTwo.money += 1
            await trx.where('id', accOne.id).update(accOne).into('account')
            await trx.where('id', accTwo.id).update(accTwo).into('account')

        } catch (err) {
            await console.log(err)  // <--- exception which was thrown is consumed here
        }
        // function returns successfully so commit will be called for the transaction
    })

do it like this:

    knex.transaction(async function (trx) {
            let objs = await trx.select('id', 'money').from('account')
            const accOne = objs[0]
            const accTwo = objs[1]

            accOne.money -= 1
            accTwo.money += 1
            await trx.where('id', accOne.id).update(accOne).into('account')
            await trx.where('id', accTwo.id).update(accTwo).into('account')
    })

or

    knex.transaction(async function (trx) {

        try {
            let objs = await trx.select('id', 'money').from('account')
            const accOne = objs[0]
            const accTwo = objs[1]

            accOne.money -= 1
            accTwo.money += 1
            await trx.where('id', accOne.id).update(accOne).into('account')
            await trx.where('id', accTwo.id).update(accTwo).into('account')

        } catch (err) {
            console.log(err)
            throw err; //<-- pass exception so that it is thrown out from the callback
        }
        // execution won't get here because exception was thrown
    })

@ghost
Copy link
Author

ghost commented Nov 1, 2016

Both ways doesn't work. If i throw an error or let a error happen there is no roleback

@elhigu
Copy link
Member

elhigu commented Nov 1, 2016

It does rollback if exception leaks, you are doing something else wrong or maybe not even running the query.

Mikaels-MacBook-Pro mikaelle$ cat test.js 
let knex = require('knex')({client: 'pg', connection: 'postgres:///knex_test'});
knex.transaction(async (trx) => {
    throw new Error("Trasaction will be rolled back");
})
.then(() => console.log('Transaction was executed and committed correctly!'))
.catch(err => console.log('Transaction failed:', err.message))
.then(() => knex.destroy());

Mikaels-MacBook-Pro mikaelle$ DEBUG='knex:*' node_modules/.bin/babel-node test.js 
  knex:tx trx1: Starting top level transaction +0ms
  knex:pool INFO pool postgresql:pg:client0 - dispense() clients=1 available=0 +2ms
  knex:client acquired connection from pool: __knexUid2 +18ms
  knex:query BEGIN; +1ms
  knex:bindings undefined +0ms
  knex:pool INFO pool postgresql:pg:client0 - dispense() clients=0 available=1 +2ms
  knex:pool INFO pool postgresql:pg:client0 - dispense() clients=0 available=2 +1ms
  knex:query ROLLBACK; +2ms
  knex:bindings undefined +0ms
  knex:tx trx1: releasing connection +2ms
  knex:client releasing connection to pool: __knexUid2 +0ms
  knex:pool INFO pool postgresql:pg:client0 - dispense() clients=0 available=3 +0ms
Transaction failed: Trasaction will be rolled back
  knex:pool INFO pool postgresql:pg:client0 - draining +1ms
  knex:pool INFO pool postgresql:pg:client0 - force destroying all objects +0ms
Mikaels-MacBook-Pro mikaelle$ 

@ghost
Copy link
Author

ghost commented Nov 1, 2016

I try to call the rollback for an example if a value is equals to 0 or throw an error. Nothing works.
With the rollback call its says "Transaction query already complete" and with the error throw "rejection error"

knex.transaction(async function (trx) {

    let objs = await trx.select('id', 'money').from('account')
    const accOne = objs[0]
    const accTwo = objs[1]

    accOne.money -= 1
    accTwo.money += 1
    await trx.where('id', accOne.id).update(accOne).into('account')
    await trx.where('id', accTwo.id).update(accTwo).into('account')

    if (accOne.money <= 0) {
        trx.rollback()
    }
})

And i try the way to use knex..transacting(trx) instead of trx.

@elhigu
Copy link
Member

elhigu commented Nov 1, 2016

If you want to call trx.rollback() or trx.commit() explicitly, you must not return Promise from your transaction's callback function. It promise is returned then implicit commit / rollback will be done (documented in http://knexjs.org/#Transactions).

Functions declared as async always return Promise, so in your example you are trying to call first rollback and then your async function returns implicitly Promise.resolve(undefined) which triggers automatic transaction commit.

knex.transaction(function thisWontReturnPromiseAnymore(trx) {
  async function thisActuallyReturnsPromiseBecauseFunctionIsDeclaredAsAsync() {
    let objs = await trx.select('id', 'money').from('account')
    const accOne = objs[0]
    const accTwo = objs[1]

    accOne.money -= 1
    accTwo.money += 1
    await trx.where('id', accOne.id).update(accOne).into('account')
    await trx.where('id', accTwo.id).update(accTwo).into('account')

    if (accOne.money <= 0) {
      trx.rollback()
    }
  }

  thisActuallyReturnsPromiseBecauseFunctionIsDeclaredAsAsync();
});

@ghost
Copy link
Author

ghost commented Nov 1, 2016

Thank you. I will try it :)
But why the "Error throw" way doesnt work as well? It is because the error throw is not effect the DB and Knex doesnt handle it?

@elhigu
Copy link
Member

elhigu commented Nov 2, 2016

I think this issue is already solved, so I'm closing this thread. Before using knex with async / await one should first understand well what how async / await are mapped to Promises (and of course understand how Promises work).

If there are any specific issues how knex is working wrong with async / await please open new issue with an example code how to reproduce the problem.

@knex knex locked and limited conversation to collaborators Nov 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant