Node.js - Promise not resolving within loop

Question

Good day all,

I'm working on extracting some data out of PipeDrive's API using Axios for Node.js. The way that PipeDrive developed their API pagination is a bit different. Here is how their pagination indicator looks:

    "additional_data": {
    "pagination": {
        "start": 0,
        "limit": 100,
        "more_items_in_collection": true,
        "next_start": 100
    }
}

I need to interate through all pages to extract the data, and my code has successfully done that, but I cannot get my promise to resolve for some reason.

My logic in the code is as follows:

if(more_items_in_collection){Add current page's data then re-run the same function with the next_start as a parameter}
else{Add current page's data then RESOLVE the promise to complete the function}

But this resolution never happens, even though my code works (strange).

Gurus, can you please take a look at my code and let me know why you think it won't resolve (function().then((result) => {OUTPUT}) never returns happens)?

Thanks as always!

 const queryPipeDrive = (start) =>{
 // Query is a full then-able Async function
 return new Promise((resolve, reject) => {
     // API CALL
     axios({
         method: 'GET', 
         url: pipeDriveURI, 
         params: {
             api_token: apiKey, 
             start: start
         }
     })
     // THEN DO THIS
     .then((response) => {
         // IF there are more items in collection on additional pages, iterate through the pages
         if(response.data.additional_data.pagination.more_items_in_collection){
             // Add all deals on page to the container
             for (const deal of response.data.data) {
                 db.get('deals').push(deal) // Push deal data to the StormDB File
             }
             
             console.log(chalk.cyan(`${response.data.additional_data.pagination.next_start}`))
             // Function loop created. We will loop UNTIL the 'more_items_in_collection' prop is false, then we'll resolve the promise. 
             queryPipeDrive(response.data.additional_data.pagination.next_start)

         }else{
             // Add all deals on this page to the reponse container
             for (const deal of response.data.data) {
                 db.get('deals').push(deal)
             }
             db.save() // Save changes to temp DB
             resolve(response.data.data) // Resolve Promise with the data from the successful call
         }
     })
     .catch((err) => {
         console.log(chalk.red(err))
         reject(err)
     })
 })

}

@Keith that's definitely on the right track, but returning the promise won't be enough, since they've wrapped this code in an extra promise — Nicholas Tower
– Nicholas Tower, Commented Jul 9, 2021 at 13:20
@NicholasTower Yeah, just went to make a coffee, while I was gone remembered he had also used a promise constructor and was about to update. — Keith
– Keith, Commented Jul 9, 2021 at 13:26
you need to read "What is the explicit Promise construction anti-pattern and how do I avoid it?" — Mulan
– Mulan, Commented Jul 9, 2021 at 14:49
@Mulan, just read this and it makes total sense. I did learn the return new Promise route but now that I understand the concept, returning an actual promise instead of wrapping everything up in one make so much MORE sense! Thanks! — TeaSeaPea_IP
– TeaSeaPea_IP, Commented Jul 9, 2021 at 17:11

Nicholas Tower · Accepted Answer · 2021-07-09 13:19:11Z

1

Your more_items_in_collection case never resolves the promise. It just creates a new one, then does nothing with it.

Additionally, you're making your code more complicated than it needs to be by using new Promise. Axios already returns a promise, so there's no need to explicitly create a new one. Calling .then will create a new promise automatically, which resolves to whatever value you return in the callback.

const queryPipeDrive = (start) => {
  // API CALL
  return axios({
    method: "GET",
    url: pipeDriveURI,
    params: {
      api_token: apiKey,
      start: start,
    },
  })
    // THEN DO THIS
    .then((response) => {
      // IF there are more items in collection on additional pages, iterate through the pages
      if (response.data.additional_data.pagination.more_items_in_collection) {
        // Add all deals on page to the container
        for (const deal of response.data.data) {
          db.get("deals").push(deal); // Push deal data to the StormDB File
        }

        console.log(
          chalk.cyan(`${response.data.additional_data.pagination.next_start}`)
        );
        // Function loop created. We will loop UNTIL the 'more_items_in_collection' prop is false, then we'll resolve the promise.
        return queryPipeDrive(
          response.data.additional_data.pagination.next_start
        );
      } else {
        // Add all deals on this page to the reponse container
        for (const deal of response.data.data) {
          db.get("deals").push(deal);
        }
        db.save(); // Save changes to temp DB
        return response.data.data;
      }
    })
    .catch((err) => {
      console.log(chalk.red(err));
      throw err;
    });
};

answered Jul 9, 2021 at 13:19

Nicholas Tower

87k10 gold badges115 silver badges127 bronze badges

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

3 Comments

TeaSeaPea_IP Over a year ago

Wow, this makes so much sense now! I just need to understand one thing, and hopefully you can help me wrap my head around it: I need this function to be Then-able so I can run more code ONCE this function is completed successfully. Is that possible without wrapping the entire function within a promise?

Nicholas Tower Over a year ago

It is thenable. axios returns a promise, and calling .then or .catch on a promise also returns a promise. You just need to return the promise, which i did on line 2 of my example.

TeaSeaPea_IP Over a year ago

You are a literal genius! Thanks so much! This works beautifully! It Really helped my understanding of Promises!

ikhvjs · Accepted Answer · 2021-07-09 13:52:27Z

Besides the accepted answer.

Would you consider using this async function in await? In this way, you call the main().

const main = async start => {
  const res = await queryPipeDrive(start);
  if (res.isMoreItems === true) {
    await main(res.nextStart);
  }
};

async function queryPipeDrive(start) {
  const response = await axios({
    method: "GET",
    url: pipeDriveURI,
    params: {
      api_token: apiKey,
      start: start,
    },
  });

  for (const deal of response.data.data) {
    db.get("deals").push(deal);
  }

  if (response.data.additional_data.pagination.more_items_in_collection) {
    console.log(
      chalk.cyan(`${response.data.additional_data.pagination.next_start}`)
    );
    return {
      isMoreItems: true,
      nextStart: response.data.additional_data.pagination.next_start,
    };
  } else {
    db.save(); // Save changes to temp DB
    return {
      isMoreItems: false,
    };
  }
}

Collectives™ on Stack Overflow

Node.js - Promise not resolving within loop

2 Answers 2

3 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related