knex / knex

A query builder for PostgreSQL, MySQL, CockroachDB, SQL Server, SQLite3 and Oracle, designed to be flexible, portable, and fun to use.
https://knexjs.org/
MIT License
18.96k stars 2.1k forks source link

Transaction Commit/Rollback events #1641

Open wubzz opened 7 years ago

wubzz commented 7 years ago

There's been discussion about this in the past, and I believe knex needs a way to support this. Using query-response is not only unreliable for this, but also not possible. It seems as though these queries do not emit the event, for whatever reason. (Debug later)

My temporary solution was to overwrite the commit function of the Transaction class in my handler that creates a knex instance.

Db.client.Transaction.prototype.commit = function(conn, value) {
            return this.query(conn, 'COMMIT;', 1, value)
                .then(() => {
                    this.trxClient.emit('trx-comitted');
                    return Promise.resolve();
                });
        };

@tgriesser I know there's plans to rewrite a lot of how transactions work, and this could be something to keep in mind. In my particular case, I'm trying to log queries within a transaction, if the transaction was a success.

tgriesser commented 7 years ago

Thanks for bringing this up, the current plan is to introduce a new concept called a "Context": a boundary for running multiple queries on the same pooled connection. All queries across an explicit context will be logged with additional info like how long the query took. More details here: https://github.com/tgriesser/knex/pull/1659

Transactions will become a specialized context, so we shouldn't need to add additional events for this case:

const trx = knex.transaction()

try {
  await trx.insert($values).into('table')
  await trx.select('*').from('table').where('id', '>', 10)
  return await trx.commit()
} catch (e) {
  return await trx.rollback()
} finally {
  const sqlLog = trx.getExecutionLog()
  console.log(sqlLog)
  /* [
    {sql: 'BEGIN TRANSACTION', bindings, time}, 
    {sql: 'INSERT INTO table VALUES ...', bindings, time}, 
    {sql: 'SELECT * FROM table WHERE id > 10', bindings, time}, 
    {sql: 'COMMIT', time}
  ] */
}
Znarkus commented 7 years ago

Hello! What is the status on this? :) Would be awesome to have for inner functions to execute code after a transaction is commited.

await doStuff1(trx)
await doStuff2(trx)
await doStuff3(trx)

function doStuff2 (trx) { 
  trx.on('commit', () => { notifyMessageQueue() })
}
kirrg001 commented 6 years ago

Is there a known workaround? e.g. can i somehow figure out the status of a transaction.

ivolucien commented 6 years ago

As of version 0.11 of knex, trx.commit().then(...) is still not a reliable way to determine if a commit has completed. Fast queries run in the .then(...) will sometimes execute before the db server has finished.

Have there been any improvements to that situation? An event as requested here would be a great feature if it reliably occurred after the commit was final on the server.

elhigu commented 6 years ago

@ivolucien on which DB? pretty much every DB has different implementation for commit. Also code to reproduce your findings would be useful.

thanpolas commented 6 years ago

@tgriesser any ETA on when 1.0 and the new transaction patterns will land?

devinivy commented 5 years ago

I also have a use-case well-summarized by @Znarkus's comment: https://github.com/tgriesser/knex/issues/1641#issuecomment-287775668

Related issue: https://github.com/tgriesser/knex/issues/1521

jbrumwell commented 5 years ago

This is the plugin we used as a workaround to ensure our neo4j server and knex transactions were linked;

import _ from 'lodash';
import Promise from 'bluebird';

export function wrapTransaction(trx, transaction) {
  const eventNames = text => text.split(/\s+/);
  let isCompleted = false;
  const methods = ['commit', 'rollback'];

  methods.forEach(name => {
    const method = trx[name];

    trx[name] = function() {
      const completed = isCompleted;

      isCompleted = true;

      return completed
        ? null
        : trx
            .triggerThen(name)
            .catch(err => {
              this._graphTransaction = null;
              transaction._rejecter(err);
              throw err;
            })
            .then(() => {
              this._graphTransaction = null;
              return method.apply(this, arguments);
            });
    }.bind(trx);
  });

  trx.triggerThen = function(names, ...args) {
    names = eventNames(names);
    const flatMap = _.flow(
      _.map,
      _.flatten
    );
    const listeners = flatMap(names, _.bind(this.listeners, this));

    return Promise.map(listeners, listener => {
      return listener.apply(this, args);
    });
  };

  trx.getGraphTransaction = function() {
    return this._graphTransaction;
  };

  trx.setGraphTransaction = function(tx) {
    this._graphTransaction = tx;

    return this;
  };

  return trx;
}

export default Bookshelf => {
  const transaction = Bookshelf.transaction.bind(Bookshelf);

  Bookshelf.transaction = cb => {
    const instance = transaction(trx => {
      trx = wrapTransaction(trx, instance);

      return cb(trx);
    });

    return instance;
  };
};

We removed our neo4j requirement so no guarantee this is still working as is with current knex bookshelf

greg-hornby-roam commented 4 years ago

Was a commit event ever implemented?

kibertoad commented 4 years ago

@elhigu Was it part of new tarn events?

elhigu commented 4 years ago

No. Tarn doesnt know anything about transactions or type of queries sent. But one can hook to to query event and probably check if it is commit. Or to hook to single transactions result promise.

greg-hornby-roam commented 4 years ago

But one can hook to to query event and probably check if it is commit.

Oh I didn't think of that. I think that could work

vladyslavarkavenko commented 2 years ago

If someone is just looking for how to just execute anything right after the transaction finish you can use this transaction.executionPromise.then(() => {})

elhigu commented 2 years ago

@VladislavArkavenko you don't need executionPromise for that... knex.transaction() returns a promise.

knex.transaction(async trx => {})
  .then(() => console.log('transaction complete'))
  .catch(err => console.log('Trx was rolled back', err)); 

... or the same with async / await...

Recodify commented 1 year ago

In case it's useful for anyone else, I did the following.

knex version: 0.21.17 (the location of the files that need patching has changed on later versions, so update them accordingly)

The below will also address this: https://github.com/knex/knex/issues/5296

  public monkeyPatchTransactionEvents(knex: any){        
        const Transaction = require('knex/lib/transaction');

        Transaction.prototype.commit = function (conn, value) {
            return this.query(conn, 'COMMIT;', 1, value)
                .then(() => {
                    knex.emit('committed', this.txid);
                    return Promise.resolve();
                }).finally(() => {
                    if (this.isCompleted()) {
                        delete conn.__knexTxId;
                    }
                });
        };

        Transaction.prototype.rollback = function (conn, error) {
            const timeout = require('knex/lib/util/timeout');
            return timeout.timeout(this.query(conn, 'ROLLBACK', 2, error).then(() => {
                knex.emit('rollback', this.txid);
                return Promise.resolve();
            }).finally(() => {
                delete conn.__knexTxId;
            }), 5000).catch(
                (err) => {
                    if (!(err instanceof KnexTimeoutError)) {
                        return Promise.reject(err);
                    }
                    return this._rejecter(error);
                }
            );
        }
    }