Node return BluebirdJS Promise

Question

I have a small problem, this script works perfectly, with one problem, the "runTenant" method is not returning a promise (that needs resolving from "all()".

This code:

Promise.resolve(runTenant(latest)).then(function() {
  end();
});

Calls this code:

function runTenant(cb) {
  return new Promise(function() {
    //global var
    if (!Tenant) {
      loadCoreModels();
      Tenant = bookshelf.core.bs.model('Tenant');
    }

    new Tenant().fetchAll()
      .then(function(tenants) {

        if (tenants.models.length == 0) {
          return;
        } else {
          async.eachSeries(tenants.models, function(tenant, next) {

            var account = tenant.attributes;
            Promise.resolve(db_tenant.config(account)).then(function(knex_tenant_config) {
              if (knex_tenant_config) {
                db_tenant.invalidateRequireCacheForFile('knex');
                var knex_tenant = require('knex')(knex_tenant_config);
                var knex_pending = cb(knex_tenant);
                Promise.resolve(knex_pending).then(function() {
                  next(null, null);
                });
              } else {
                next(null, null);
              }
            });

          });
        };
      });
  });
}

The code from runTenant is working correctly however it stalls and does not proceed to "end()" because the promise from "runTenant(latest)" isn't being resolved.

As if it weren't apparent, I am horrible at promises. Still working on getting my head around them.

Many thanks for any help/direction!

score 1 · Accepted Answer · edited May 23 '17 at 10:28

1

You should not use the Promise constructor at all here (and basically, not anywhere else either), even if you made it work it would be an antipattern. You've never resolved that promise - notice that the resolve argument to the Promise constructor callback is a very different function than Promise.resolve.

And you should not use the async library if you have a powerful promise library like Bluebird at hand.

As if it weren't apparent, I am horrible at promises.

Maybe you'll want to have a look at my rules of thumb for writing promise functions.

Here's what your function should look like:

function runTenant(cb) {
  //global var
  if (!Tenant) {
    loadCoreModels();
    Tenant = bookshelf.core.bs.model('Tenant');
  }
  return new Tenant().fetchAll().then(function(tenants) {
    // if (tenants.models.length == 0) {
    //  return;
    // } else
    // In case there are no models, the loop iterates zero times, which makes no difference
    return Promise.each(tenants.models, function(tenant) {
      var account = tenant.attributes;
      return db_tenant.config(account).then(function(knex_tenant_config) {
        if (knex_tenant_config) {
          db_tenant.invalidateRequireCacheForFile('knex');
          var knex_tenant = require('knex')(knex_tenant_config);
          return cb(knex_tenant); // can return a promise
        }
      });
    });
  });
}

edited May 23 '17 at 10:28

Community

1
1

answered Apr 05 '16 at 21:22

Bergi

630,263
148
957
1,375

Thank you @Bergi, that worked perfectly. I'll read through your rules of thumb as well, as I would like to understand more clearly... as to what is exactly is happening. I was under the impression that the first code calling the runTenant since it was in a Resolve method, that any promise returned to it would be resolved, the primary thing I notice with your suggestion (which worked perfectly) is that you resolved all promises before the return. ben above, also recommended this. Thank you both! – sol Apr 06 '16 at 04:34
Ah, yeah, forgot to include a remark about your confusion with `resolve`. Feel free to comment about every part you don't understand, I'll clarify those. – Bergi Apr 06 '16 at 10:30
I'm not even sure whether you need that `Promise.resolve(db_tenant.config(account))` - what does `config(…)` return? – Bergi Apr 06 '16 at 10:31
config returns a promise as well – sol Apr 13 '16 at 22:48
I see, in that case you can savely omit the `Promise.resolve` call – Bergi Apr 14 '16 at 00:37

Ben · Answer 2 · 2016-04-06T16:16:18.510

You need to return all the nested promises. I can't run this code, so this isn't a drop it fix. But hopefully, it helps you understand what is missing.

function runTenant(cb) {
    //global var
    if (!Tenant) {
        loadCoreModels();
        Tenant = bookshelf.core.bs.model('Tenant');
    }

    return new Tenant().fetchAll() //added return
        .then(function (tenants) {

            if (tenants.models.length == 0) {
                return;
            } else {
                var promises = []; //got to collect the promises
                tenants.models.each(function (tenant, next) {

                    var account = tenant.attributes;
                    var promise = Promise.resolve(db_tenant.config(account)).then(function (knex_tenant_config) {
                        if (knex_tenant_config) {
                            db_tenant.invalidateRequireCacheForFile('knex');
                            var knex_tenant = require('knex')(knex_tenant_config);
                            var knex_pending = cb(knex_tenant);
                            return knex_pending; //return value that you want the whole chain to resolve to
                        }
                    });
                    promises.push(promise); //add promise to collection
                });
                return Promise.all(promises); //make promise from all promises
            }
        });
}

Also, it looks like you are using built-in Promises. I highly recommend you use [Bluebird](http://bluebirdjs.com/) instead. It has built in method like Promise.each() that would make this easier. It also has a helpful warning system that would have warned you about some of the errors here. — Ben, Apr 05 '16 at 20:34

score 0 · Answer 3 · answered Apr 05 '16 at 20:35

Your promise in runTenant function is never resolved. You must call resolve or reject function to resolve promise:

function runTenant() {
  return new Promise(function(resolve, reject) {
    // somewhere in your code
    if (err) {
      reject(err);
    } else {
      resolve();
    }
  });
});

And you shouldn't pass cb in runTenant function, use promises chain:

runTenant()
 .then(latest)
 .then(end)
 .catch(function(err) {
   console.log(err);
 });

Node return BluebirdJS Promise

3 Answers3