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, Jul 09 '21 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, Jul 09 '21 at 13:26
you need to read [_"What is the explicit Promise construction anti-pattern and how do I avoid it?"_](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it) — Mulan, Jul 09 '21 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, Jul 09 '21 at 17:11

score 1 · Accepted Answer · answered Jul 09 '21 at 13:19

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;
    });
};

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? — TeaSeaPea_IP, Jul 09 '21 at 13:22
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. — Nicholas Tower, Jul 09 '21 at 13:23
You are a literal genius! Thanks so much! This works beautifully! It Really helped my understanding of Promises! — TeaSeaPea_IP, Jul 09 '21 at 13:31

ikhvjs · Answer 2 · 2021-07-09T13:52:27.157

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,
    };
  }
}

Node.js - Promise not resolving within loop

2 Answers2