Node.js Promise Chain with API and database query

Question

I am pretty new to the whole promises business and am trying out my project with them.
No I need to query an external API, go several items in the response, do other checks on my own database with them and when all of them are finished, need to resolve the whole promise in itself.
I also need even another call to my database at the marked point with another promise. These are independent from each other but the "parent" promise should only resolve when both are resolved.
I am at a wall here and probably need some general explanation about chaining several promises with multiple items.
Maybe I just understand something generally wrong here... Here is my code so far (shortened at ...):

'use strict';

import rp from 'request-promise';
import _ from 'lodash';

var Import = {
  init: function(user, options) {
    return new Promise((resolve, reject) => {
      rp.get("...") // call to external API
        .then((res) => {
          ...
          resolve();
        });
      ...
    });
  },
  run: function() {
    return new Promise((resolve, reject) => {
      ...

      rp.get(...) // call to external API
        .then((res) => {
          var events = JSON.parse(res).data;
          var promises = [];

          for (var i = 0; i < events.length; i++) {

            promises.push(new Promise((resolve, reject) => {
              ...

              Location.findOneAndUpdateAsync(...)
                .then((loc) => {
                  events[i].location = loc._id;
                  resolve();
                })
                .catch((err) => {
                  console.error(err);
                  reject();
                });

              // I need even another call to my database here later with another promise
            }));
          }
          return Promise.all(promises)
            .then(() => {
              console.log("all promises resolved");
              resolve(events);
            });
        })
        .catch((err) => {
          console.error(err);
          reject(err);
        });
    });
  }
};

it doesn't help to hide your question away inside the comments to the code... You're also using the typical promise "anti-pattern" of creating a new promise in your loop around a function that already creates a promise. — Alnitak
– Alnitak, Commented Feb 15, 2016 at 15:38
These was just supposed to be an addition but I'm gonna change that — Kageetai
– Kageetai, Commented Feb 15, 2016 at 15:41
First of all, avoid the Promise constructor antipattern. Your functions already return promises. — Bergi
– Bergi, Commented Feb 15, 2016 at 15:41
Sounds like you're looking for Promise.all([onePromise, otherIndependentPromise]).then(whenBothAreDone) — Bergi
– Bergi, Commented Feb 15, 2016 at 15:43
@Kageetai is that two calls you're expecting to make inside that loop? — Alnitak
– Alnitak, Commented Feb 15, 2016 at 15:46

Community · Accepted Answer · 2017-05-23 12:23:21Z

3

You can dramatically simplify your code by not falling foul of the Promise constructor anti-pattern - the functions you're calling already return promises so you should take advantage of those.

Then, eliminate the for / push loop with Array.prototype.map:

var promises = events.map(function(event) {
    return Location.findOneAndUpdateAsync(...)
            .then((loc) => event.location = loc._id);
});

You said you have two sets of calls to make, but as they're independent, you might as well just use another .map call:

var promises2 = events.map(function(event) {
    return ...
});

and then you just need to wait for all of those and return your events object:

return Promise.all(promises.concat(promises2)).then(() => events);

There's no need for all those .catch blocks - you should let the error propagate upwards.

If (per your comments) the two inner calls have a dependency, you could try this:

var promises = events.map(function(event) {
    return Location.findOneAndUpdateAsync(...)
           .then((loc) => {
               event.location = loc._id;
               if (condition) {
                   return XXXOtherFunctionReturningPromise();
               }
           });
});

[and obviously then eliminate the .concat call from above]

edited May 23, 2017 at 12:23

CommunityBot

11 silver badge

answered Feb 15, 2016 at 15:54

Alnitak

341k72 gold badges420 silver badges503 bronze badges

Sign up to request clarification or add additional context in comments.

8 Comments

Kageetai Over a year ago

Hmm, would it be as easy to still the two calls depended, in the way, that I want to skip an event entirely if the first (which I still have to implement) call (to my own database) returns a specific answer?

Alnitak Over a year ago

probably easy, but not what you asked for! ;-)

Kageetai Over a year ago

Yeah, my own fault, just re-thought the whole process a bit... X-)

Alnitak Over a year ago

I've added something, but now on re-reading your comment I'm not sure what you're asking for. Was the first call the "first one in the loop", or the rp.get() call ?

Kageetai Over a year ago

I meant where I did Location.findOneAndUpdateAsync() was supposed be the first call to my own database and I still need another one afterwards to my database again

|

Collectives™ on Stack Overflow

Node.js Promise Chain with API and database query

1 Answer 1

8 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

8 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related