How can I use this with functions already returning a promise?

[masimplo]

Hello,

I am using Bluebird to fetch data from a graph database, whose driver also uses Bluebird, so that you execute queries using promises instead of callbacks.

My question is how can I use your module to wrap this?

for example I have this function that fetches all Groups from the db:

    groups.getAll = function () {
        return database.getDb()
            .then(function (db) {
                return db
                    .select('*, out(\'HasUser\').size() as numOfUsers')
                    .from('Group')
                    .all()
                    .then(function (groups) {
                        var groupVms = [];
                        groups.forEach(function (group) {
                            groupVms.push(new Group(group));
                        });
                        return groupVms;
                    });
            });
    };

I tried doing something like:

groups.getAll = new CircuitBreaker(groups.getAll, null, {
       timeout: 1000
    }).exec;

but I always get a timeout after 1 sec. When stepping through the code on runtime I see that return groupVms line is hit but then nothing is returned on groups.getAll().then() and I am guessing is has to do with the fact that there is no callback to call to pass back the data.

[masimplo]

There is a workaround I could use, documented in Bluebird documentation, which is to use a hybrid form of function where you do pass a callback as the last argument and when returning the promise you first append .nodify(callback) to the returned promise. If a callback is passed then it called, otherwise the normal promise flow is followed.

Problem is that this has a performance hit since you are using callbacks instead of promises when wrapping with your circuit breaker. It would be more efficient if promises were used all the way.

[pablolb]

Hi Michael,

First of all, let me say I worked on this for a project but I never got the chance to use it in production before I left. You should go over the tests and code to see if it suits your needs. That is you won't see any activity on this.

Having said that, it is true, I did not think of the case for using an "already promisified" function, only the typical node callback/errback ones. You could do that, to nodify back the function, or change the code... you'll see that the protected function is called at the end of the runRequest somehow...

https://github.com/pablolb/promise-circuitbreaker/blob/master/lib/circuitbreaker.js#L533

// this is still "nodifying" fn().then(cb).catch(function(err) {cb(err); });

The point of the module is to protect against timeouts and fail fast once the underlying function is "offline/broken" - so the code uses is a timeout for this, to check if the function called-back or not. You'd still be having two promises - at least with the solution presented in this module.

On Fri, May 8, 2015 at 1:32 PM, Michael [email protected] wrote:

There is a workaround I could use, documented in Bluebird documentation, which is to use a hybrid form of function where you do pass a callback as the last argument and when returning the promise you first append .nodify(callback) to the returned promise. If a callback is passed then it called, otherwise the normal promise flow is followed.

Problem is that this has a performance hit since you are using callbacks instead of promises when wrapping with your circuit breaker. It would be more efficient if promises were used all the way.

— Reply to this email directly or view it on GitHub https://github.com/pablolb/promise-circuitbreaker/issues/1#issuecomment-100218402 .

[masimplo]

Hi Pablo,

I really appreciate you taking the time and responding to this.

Since you are no longer maintaining this repo I will probably fork it and make changes to the code to use promises.

I didn't have the chance of going through your code yet, but I was thinking of using bluebird timeout function on the passed promise to have it reject if not completed within the given timeout rather than setting a separate timeout. In any case I will give it a go and see what turns up.