Constraint violations during commit are lost

[knex]

Constraint violations during commit are lost

[tgriesser]

So the issue here is that the commits are failing, but the transaction block is resolving with a successful promise? That seems odd, can you paste some code showing a brief example of how you have the transaction setup so I can get a better idea of what the issue might be?

[studds]

That's right, the transaction block is resolving with a successful promise but the commit is actually failed. I've mocked up a simple example:

(function () {

    var query = 'drop schema if exists transdemo cascade;' +
        'create schema transdemo;' +
        'CREATE TABLE "transdemo"."one" (' +
        '"id" int4 NOT NULL,' +
        'PRIMARY KEY ("id")' +
        ')' +
        'WITH (OIDS=FALSE);' +
        'CREATE TABLE "transdemo"."two" (' +
        '"id" int4 NOT NULL,' +
        '"oneId" int4 NOT NULL,' +
        'PRIMARY KEY ("id")' +
        ')' +
        'WITH (OIDS=FALSE);' +
        'ALTER TABLE "transdemo"."two" ADD CONSTRAINT "two_one_fk"' +
        'FOREIGN KEY ("oneId") REFERENCES "transdemo"."one" ("id")' +
        'ON UPDATE NO ACTION ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED;';

    var Bookshelf = require('bookshelf');
    var connectionString = process.env.DATABASE_URL || 'postgres://localhost:5432/traceapp';
    var pg = require('pg');
    var Client = new pg.Client(connectionString);

    Bookshelf.Initialize({
        client: 'pg',
        connection: {
            user: Client.connectionParameters.user,
            database: Client.connectionParameters.database,
            port: Client.connectionParameters.port,
            host: Client.connectionParameters.host,
            password: Client.connectionParameters.password,
            binary: Client.connectionParameters.binary,
            ssl: Client.connectionParameters.ssl,
            charset  : 'utf8'
        },
        debug: true
    });

    return Bookshelf.Knex.Raw(query)
        .then(function () {
            var One = Bookshelf.Model.extend({
                tableName: 'transdemo.one'
            });
            var Two = Bookshelf.Model.extend({
                tableName: 'transdemo.two'
            });

            return Bookshelf.Transaction(function (t) {
                Two.forge({id: 1, oneId: 1})
                    .save(null, {transacting: t})
                    .then(function () {
                        console.log('save succeeds because constraint is deferred');
                        t.commit();
                    })
                    .otherwise(function (err) {
                        console.log('save has failed', err, err.stack);
                        t.rollback();
                    });
            })
                .then(function () {
                    console.log('transaction has succeeded, so should be a row in two');
                    return Two.forge({id: 1})
                        .fetch()
                        .then(function (two) {
                            if (two) {
                                console.log('huh, how did that get created?');
                            } else {
                                console.log('transaction was successful, but the rows really were not created');
                            }
                        })
                        .otherwise(function (err) {
                            console.error(err, err.stack);
                        });
                })
                .then(function () {
                    process.exit(0);
                })
                .otherwise(function () {
                    process.exit(1);
                });

        })
        .otherwise(function () {
            process.exit(1);
        });

})();

The results I get are:

{ sql: 'drop schema if exists transdemo cascade;create schema transdemo;CREATE TABLE "transdemo"."one" ("id" int4 NOT NULL,PRIMARY KEY ("id"))WITH (OIDS=FALSE);CREATE TABLE "transdemo"."two" ("id" int4 NOT NULL,"oneId" int4 NOT NULL,PRIMARY KEY ("id"))WITH (OIDS=FALSE);ALTER TABLE "transdemo"."two" ADD CONSTRAINT "two_one_fk"FOREIGN KEY ("oneId") REFERENCES "transdemo"."one" ("id")ON UPDATE NO ACTION ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED;',
  bindings: [],
  __cid: '__cid1' }
{ sql: 'update "transdemo"."two" set "id" = $1, "oneId" = $2 where "id" = $3',
  bindings: [ 1, 1, 1 ],
  __cid: '__cid2' }
save succeeds because constraint is deferred
transaction has succeeded, so should be a row in two
{ sql: 'select * from "transdemo"."two" where "id" = $1 limit 1',
  bindings: [ 1 ],
  __cid: '__cid3' }
transaction was successful, but the rows really were not created

[tgriesser]

So it looks like the issue here might not be with the transaction, but rather the fact that if you set the primary key, Bookshelf assumes you're going for an update... I think you're looking to insert without having it create the primary key for you, you'll have to do:

Two.forge({id: 1, oneId: 1}).save(null, {transacting: t, method: 'insert'});

Let me know if that isn't the source of the issue.

[studds]

Ahh, woops, normally I'm overriding new() and using the serverCreatedAt instead of id (as per bookshelf/bookshelf#24), but I forgot about that in this example.

And, it turns out, the problem is completely different to what I thought!

The promise is actually not resolved at all. I thought it was resolving because of an error in my main code.

Updated example:

(function () {

    var query = 'drop schema if exists transdemo cascade;' +
        'create schema transdemo;' +
        'CREATE TABLE "transdemo"."one" (' +
        '"id" int4 NOT NULL,' +
        '"serverCreatedAt" timestamp NOT NULL,' +
        '"serverUpdatedAt" timestamp NOT NULL,' +
        'PRIMARY KEY ("id")' +
        ')' +
        'WITH (OIDS=FALSE);' +
        'CREATE TABLE "transdemo"."two" (' +
        '"id" int4 NOT NULL,' +
        '"oneId" int4 NOT NULL,' +
        '"serverCreatedAt" timestamp NOT NULL,' +
        '"serverUpdatedAt" timestamp NOT NULL,' +
        'PRIMARY KEY ("id")' +
        ')' +
        'WITH (OIDS=FALSE);' +
        'ALTER TABLE "transdemo"."two" ADD CONSTRAINT "two_one_fk"' +
        'FOREIGN KEY ("oneId") REFERENCES "transdemo"."one" ("id")' +
        'ON UPDATE NO ACTION ON DELETE NO ACTION DEFERRABLE INITIALLY DEFERRED;';

    var Bookshelf = require('bookshelf');
    var connectionString = process.env.DATABASE_URL || 'postgres://localhost:5432/traceapp';
    var pg = require('pg');
    var Client = new pg.Client(connectionString);

    Bookshelf.Initialize({
        client: 'pg',
        connection: {
            user: Client.connectionParameters.user,
            database: Client.connectionParameters.database,
            port: Client.connectionParameters.port,
            host: Client.connectionParameters.host,
            password: Client.connectionParameters.password,
            binary: Client.connectionParameters.binary,
            ssl: Client.connectionParameters.ssl,
            charset  : 'utf8'
        },
        debug: true
    });

    // A model is new if it has never been saved to the server, and lacks an 'serverCreatedAt' date
    Bookshelf.Model.prototype.hasTimestamps = ['serverCreatedAt', 'serverUpdatedAt'];
    Bookshelf.Model.prototype.isNew = function () {
        return !this.get('serverCreatedAt');
    };

    return Bookshelf.Knex.Raw(query)
        .then(function () {
            var One = Bookshelf.Model.extend({
                tableName: 'transdemo.one'
            });
            var Two = Bookshelf.Model.extend({
                tableName: 'transdemo.two'
            });
            var Twos = Bookshelf.Model.extend({

            });

            return Bookshelf.Transaction(function (t) {
                Two.forge({id: 1, oneId: 1})
                    .save(null, {transacting: t})
                    .then(function () {
                        console.log('save succeeds because constraint is deferred');
                        t.commit();
                    })
                    .otherwise(function (err) {
                        console.log('save has failed', err, err.stack);
                        t.rollback();
                    });
            })
                .then(function () {
                    console.log('transaction has succeeded, so should be a row in two');
                    return Two.forge({id: 1})
                        .fetch()
                        .then(function (two) {
                            if (two) {
                                console.log('huh, how did that get created?');
                            } else {
                                console.log('transaction was successful, but the rows really were not created');
                            }
                        })
                        .otherwise(function (err) {
                            console.error(err, err.stack);
                        });
                })
                .then(function () {
                    process.exit(0);
                });

        })
        .otherwise(function (err) {
            console.log('transaction failed', err, err.stack);
            process.exit(1);
        });

})();

Run normally, it hangs forever because the promise doesn't resolve. If, on the other hand, I add an 'otherwise' in finishTransaction in knex/clients/base.js:

  finishTransaction: function(type, trans, dfd, msg) {
    var ctx = this;
    nodefn.call(trans.connection.query.bind(trans.connection), type + ';', []).then(function(resp) {
        console.log(resp);
      if (type === 'commit') dfd.resolve(msg || resp);
      if (type === 'rollback') dfd.reject(msg || resp);
    })
        .otherwise(function (err) {
            dfd.reject(err);
        })
        .ensure(function() {
      ctx.releaseConnection(trans.connection);
      trans.connection = null;
    });
  }

... then it works a treat.