Knex transaction Promises warnings

Question

I am having a hard time working with Knex, transactions and Promises and would appreciate the help of the experts of SO :)

From reading similar posts and many many articles on Promises, I understand that I am not too far from the solution, but find that the following code is really overly complicated for such a simple task.

This class simply exposes a function that receives a query that was built previously and needs to run it after first calling a "load_user_settings" function (using PostgreSQL 9.5 as a DB). I actually need the result (or error) of the 2nd query, so simply putting trx.commit in the "then" does not work for me.

The process.nextTick at the end of my function really feels like a huge hack and was added in order to avoid changing my entire application to Promises since that is not doable at the moment. Without that, I often found myself with Promise warnings ("Unhandled rejection Error") and hanging code when an exception was thrown later in my code.

Also, I can see the following warning in my logs from time to time : Warning: a promise was rejected with a non-error: [object Undefined] and I guess that that warning is somehow related to this problem...

If a solution using only callbacks could be found, that would also be great, as our entire application is currently using callbacks.

const pg = require('pg');
const async = require('async');

class MyClass {

    constructor(connectionInfo) {
        this.connectionInfo = connectionInfo; // This is an object containing user, password, database, host and port.

        this.pgKnex = require('knex')({
            client : 'pg',
            connection : this.connectionInfo,
            pool : {
                min : 1,
                max : 25
            }
        });
    }

    query(username, sql, params, callback) {
        let response;
        let error;

        this.pgKnex.transaction((trx) => {
            return this.pgKnex.raw(`SELECT load_user_settings(?)`, [username]).transacting(trx)
                .then(() => {
                    return this.pgKnex.raw(sql, params).transacting(trx);
                })
                .then((result) => {
                    response = result;
                    return trx.commit();
                }).catch((err) => {
                    error = err;
                    console.error(err);
                    return trx.rollback();
                });
        }).catch((err) => {
            error = err;
            console.error(err);
        }).then(() => {
            process.nextTick(() => {
                callback(error, response);
            });
        });
    }
}

module.exports = MyClass;

Following the comment made by Bergi, I wrote this version which avoids any hacks and removes all the promise warnings that I had :

query(username, sql, params, callback) {
    this.pgKnex.transaction((trx) => {
        this.pgKnex.raw(`SELECT set_config('ims.username', ?, false)`, [username]).transacting(trx).asCallback((err, result) => {
            if(err) {
                trx.rollback().asCallback(() => {
                    console.error(err);
                    callback(err);
                });
                return;
            }

            this.pgKnex.raw(sql, params).transacting(trx).asCallback((err, result) => {
                if(err) {
                    trx.rollback().asCallback(() => {
                        console.error(err);
                        callback(err);
                    });
                    return;
                }

                trx.commit().asCallback((commitErr) => {
                    callback(commitErr, result);
                });
            });
        });
    }).asCallback((err, result) => {
        // Nothing to do here ... I guess?
    });
}

But I still feel like I this could be improved...

Answer 1

Let me try to simplify:

query(username, sql, params, callback) {
    this.pgKnex.transaction((trx) => {
        return this.pgKnex.raw(`SELECT set_config('ims.username', ?, false)`, [username])
            .transacting(trx)
            .then(() => {
                return this.pgKnex.raw(sql, params)
                    .transacting(trx);
            });
    }).asCallback(callback);
}

When using a transaction block, you can return a Promise and knex will automatically commit/rollback if the Promise is resolved/rejected.

The asCallback(callback) call at the end ensures that errors thrown within the promise chain will be passed to callback. You don't need to handle errors on each promise individually because all errors of a promise chain will bubble. Likewise, if a result is returned from the second sql query (the last of the promise chain), this result is passed as second argument to callback.