Constraint violations during commit are lost

[studds]

I'm using postgres, and I've got all my foreign key constraints set up as DEFERRABLE INITIALLY DEFERRED. This means I don't need to ensure each query in the transaction is executed in the right order to satisfy foreign key constraints, so long as the transaction overall is valid. This means that the constraint violation isn't triggered on the original query, but rather on commit.

I'm finding that commits are failing but in my app (using Bookshelf), the transaction gets resolved and not rejected.

I can't see anyway of finding out whether the commit was successful or not... should commit return a promise?

[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.