JackAdams / meteor-transactions

App level transactions for Meteor + Mongo
http://transactions.taonova.com/
MIT License
113 stars 16 forks source link

Possible? Leave failed transactions in "transactions" collection #44

Closed dheerajbhaskar closed 8 years ago

dheerajbhaskar commented 8 years ago

Is it possible to leave the failed transactions in the transactions collection?

I see no record of failed transactions except in server logs, which is pretty bad, no?

PS: If you need help, please point at the code and some idea of what needs to be done, and I can give a PR if I can manage it.

dheerajbhaskar commented 8 years ago

I started digging into the source (thank you open source), I see that rolledback transactions should not be deleted from transactions. But transactions are rolledback, but not being recorded. BTW, I'm using tx.cancel at a certain point and doing a return; right after that.

// If you want to limit the volume of rubbish in the transactions collection // you can set tx.removeRolledBackTransactions = true // It is false by default because having a record in the db helps with debugging // [BOTH]

this.removeRolledBackTransactions = false;

JackAdams commented 8 years ago

Okay. I'll look into this. They really shouldn't be removed.

dheerajbhaskar commented 8 years ago

This seems repeatable.

I20151109-05:05:11.052(5.5)? Pushed update command to stack: 6NTAiGpHAziC7SM3f I20151109-05:05:11.064(5.5)? Transaction cancelled I20151109-05:05:11.068(5.5)? Incomplete transaction removed: 6NTAiGpHAziC7SM3f

That id is not found in the collection. That id is not found in the collection.

This is the code I'm using: tx.cancel was just added to test.

api.addRoute('transfer_chips/:id', {authRequired: false}, {
  post: {
    action: function () {
      var userIdFrom      = this.urlParams.id;
      var userIdTo        = this.bodyParams.toId;
      var chipsToTransfer = this.bodyParams.chips;

      //TODO: set timeout for transactions to 30s; (or slighlty longer?)
      //TODO make bets also transactions
      //TODO set self.repair to rollback rather than complete
      // TODO put the config in server startup or the like
      // *******************
      // Transactions config
      // *******************
      tx.selfRepairMode               = 'rollback';
      tx.idleTimeout                  = 30000;
      tx.removeRolledBackTransactions = false;

      var isAccountDebitable = Users_collection.findOne({
        _id  : userIdFrom,
        chips: {$gte: chipsToTransfer}
      }, {fields: {_id: 1}});

      if (isAccountDebitable) {
        //TODO add context if possible
        //TODO you might have to change the string here because it might be serialized based on it
        //start transaction
        tx.start("transfer_chips");
        var transactionDate = new Date();
        Users_collection.update({_id: userIdFrom, chips: {$gte: chipsToTransfer}},
                                {
                                  $inc: {chips: -chipsToTransfer}, $push: {
                                  transfer_history: {
                                    date : transactionDate,
                                    from : userIdFrom,
                                    to   : userIdTo,
                                    chips: chipsToTransfer
                                  }
                                }
                                }, {tx: true});
        tx.cancel();

        Users_collection.update({_id: userIdTo}, {
          $inc: {chips: chipsToTransfer}, $push: {
            transfer_history: {
              date : transactionDate,
              from : userIdFrom,
              to   : userIdTo,
              chips: chipsToTransfer
            }
          }
        }, {tx: true});
        tx.commit();
      } else {
        //TODO base return on commit callback status

        return {status: 'failure. Couldn\'t debit chips'};
      }

      return {status: 'success'};
    }
  }
});

PS: I noticed that one cannot just return after cancel, but it has to reach the tx.commit. Having said that this issue still exists.

dheerajbhaskar commented 8 years ago

Is it possible that this might be the issue?: https://github.com/JackAdams/meteor-transactions/blob/master/lib/transactions-common.js#L220

The insert statement is commented out. I did a whole line-by-line debug, this might be it, but I might be mistaken, it's 5:30AM here :)

dheerajbhaskar commented 8 years ago

Any updates on this mate? Please do let me know if I can be of some help.

JackAdams commented 8 years ago

Still working on it. I need to write some automated tests before releasing.

JackAdams commented 8 years ago

I've just committed a new version on Github, but haven't released it on atmosphere yet, as I want to write a couple more tests first.

I think this issue is partially fixed, but it needs more work.

If you want to test it now, you could clone the Github repo into your /packages directory.

JackAdams commented 8 years ago

Just released 0.7.5 which should fix this. Let me know if this is still an issue.