Promise.all() in node doesn't return value

kkoutoup · April 26, 2019, 10:20pm

Code for the following question here: https://bit.ly/2ZC8MZ5

I am trying to write a script that does two separate API calls.

The first call gets data and then passes that to a second call to a different API.

I am trying to console.log the results of the second API call but all I’m getting is => Promise{pending} . In the browser the second call is resolved and the data can be seen in the console. However, in node the promise is pending.

Any ideas why? My code is below. Many thanks!

//dependencies
const XMLHttpRequest = require("xmlhttprequest").XMLHttpRequest;
const xhr = new XMLHttpRequest();

// variables
const astrosUrl = 'http://api.open-notify.org/astros.json';
const wikiUrl = 'https://en.wikipedia.org/api/rest_v1/page/summary/';

//getJSON
getJSON = (url) => {
  return new Promise((resolve, reject) => {
    const xhr = new XMLHttpRequest(url);
    xhr.open('GET', url);
    xhr.onload = () => {
      if(xhr.status === 200){
        let data = JSON.parse(xhr.responseText);
        resolve(data);
      }else{
        reject(new Error(xhr.statusText));
      }
    }
    xhr.send();
  })
}

//get wiki data
getProfiles = (data) =>{
  const profiles = data.people.map(person => {
    return getJSON(wikiUrl+formatName(person.name));
  })
 return Promise.all(profiles);
}

//handle data
handleData = (data) =>{
  console.log(data);
}

//HELPER FUNCTIONS
formatName = (name)=>{
  return name.split(" ").join("_");
}

//run sequence
getJSON(astrosUrl)
.then(getProfiles)
.then(handleData)
.catch(error => console.log(error.message))

dmoneyballer · April 26, 2019, 10:58pm

The function was just returning the promise. I think the best way to work with it is to call the .then from within the first function. The way I got it to work is to call the other function from the getProfiles function like this

getProfiles = (data) =>{
  const profiles = data.people.map(person => {
    return getJSON(wikiUrl+formatName(person.name));
  })
  Promise.all(profiles)
  .then(handleData(data))
 return 
}

Here’s a replit with my code. https://repl.it/repls/LameLightgreenVoxels

kkoutoup · April 27, 2019, 10:28am

Thank you very much for your help!
I am still a bit confused and that’s because my question wasn’t clear.

Revised code available here: https://repl.it/@kkoutoup/LameLightgreenVoxels

What i want to achieve is to pass the combined data stored in the const profiles variable to the handleData function (which should probably be renamed to handleProfiles to be less misleading). What your solution does is pass the data from the first API call to the handle data function.

Following your suggestion i changed the parameter of the Promise.all() as follows:

getProfiles = (data) =>{
  const profiles = data.people.map(person => {
    return getJSON(wikiUrl+formatName(person.name));
  })
  Promise.all(profiles)
  .then(handleProfiles(profiles))
 return 
}

//handle data
handleProfiles = (profiles) =>{
  console.log('These are the returned profiles', profiles);
}

However, the console.log in the handleProfiles function gives me an array of six pending promises. I’m not sure how to handle this data since the promises are not resolved. Any ideas? Again thank you!

metasean · April 27, 2019, 12:03pm

Hey kkoutoup,

The root of the problem is that getJson is itself actually returning a promise not a result, which you are then assigning (i.e. you were assigning the Promises and not the Promises’ results) to the profiles array on line 28.

There are several ways to correct this. Note that the several steps are the same for both approaches.

Approach 1: Still using `Promise.all`

If you _really need to use a Promise.all, then the easiest way is to set up a let profiles = [] at a high enough scope to be available to your functions.
When you iterate over your profile data (lines 28-29)
a. Do NOT assign the result to profiles, i.e do just

     data.people.map(person => {
     // instead of
     const profiles = data.people.map(person => {

b. Use a forEach instead of map, i.e.

       data.people.forEach(person => {
       // instead of 
       data.people.map(person => {

(This isn’t strictly necessary, but since it isn’t using the map-generated array value, it helps to makes the intent a bit easier to interpret.)

push each data value into that profiles variable

    profiles.push(person)
    // instead of 
    return getJSON(wikiUrl+formatName(person.name));

here’s a repl with that version: https://repl.it/repls/DeafeningNextMotion

Approach 2: Call `handleProfiles` at the time of data return

When you iterate over your profile data (lines 28-29)
a. Do NOT assign the result to profiles, i.e do just

       data.people.map(person => { 
       // instead of
       const profiles = data.people.map(person => {

b. Use a forEach instead of map, i.e.

       data.people.forEach(person => { 
       // instead of 
       data.people.map

(This isn’t strictly necessary, but since it isn’t using the map-generated array value, it helps to makes the intent a bit easier to interpret.)

Instead of collecting the results to pass to the Promise.all, just send them directly to handleProfiles, i.e.

    return getJSON(wikiUrl+formatName(person.name))
      .then( handleProfiles );
    // instead of 
    Promise.all(profiles)
      .then(handleProfiles(profiles))

here’s a repl with that version: https://repl.it/repls/DarkvioletPungentWordprocessor

(I formatted this the best I could, but I couldn’t get the outline formatting to display correctly . Hopefully it isn’t too confusing.)

jnmorse · April 27, 2019, 12:22pm

The XMLHttpRequest package apparently hasn’t been updated in 4 years, not sure if that is the cause of the issue, but from what you had I couldn’t figure out why it wasn’t really working either. But I tried using fetch which would be the same in the browser as in node with the node-fetch package as well. It seemed to behave the way you would have expected.

const fetch = require('node-fetch')

const astrosUrl = 'http://api.open-notify.org/astros.json'
const wikiUrl = 'https://en.wikipedia.org/api/rest_v1/page/summary'

function formatName(name) {
  return name.split(' ').join('_')
}

function getJSON (url) {
  return new Promise((resolve, reject) => {
    fetch(url)
      .then(response => response.json())
      .then(data => resolve(data))
      .catch(error => reject(error))
  })
}

function getProfiles(data) {
  const profiles = data.people.map(person => getJSON(`${wikiUrl}/${formatName(person.name)}`))
  return Promise.all(profiles)
}

function handleData(data) {
  console.log(data)
}

getJSON(astrosUrl)
  .then(getProfiles)
  .then(handleData)

kkoutoup · April 27, 2019, 2:33pm

@metasean Thank you very much! Worked like a charm!!

metasean · April 27, 2019, 2:55pm

@kkoutoup

For what it’s worth, @jnmorse’s comments about XMLHttpRequest being outdated, and fetch being the better choice are % correct!

(I got so busy trying to address why the Promise.all wasn’t working the way you were expecting, that I totally forgot to address that aspect of your approach. )

Unless fetch just isn’t an option (e.g. you have to support IE) then I’d definitely encourage you to use it instead of XMLHttpRequest!

The easiest ways to figure out if something like fetch is supported are via:

the “Browser Compatibility” section of the MDN docs
https://caniuse.com
https://kangax.github.io/compat-table/es6/ (not relevant for fetch which is part of the html standard, but great for node related apis)

kkoutoup · April 27, 2019, 7:27pm

Amazing! Thank you once again for your time, detailed explanation and resources!
At this point i’m just trying to wrap my head around promises and Promise.all() so there are no real restrictions i.e. Internet Explorer. I’ll give fetch a shot too following @jnmorse’s comments.

jnmorse · April 27, 2019, 8:05pm

Promise.all() takes an array of promises and only once all the promises that are in it resolve does .then after that take place. There is also Promise.race() which will return the first to resolve. I’ve honestly yet to find a use for Promise.race().

If for some reason you need promise.all the resolve even if some of the promises you supplied did not succed you can kinda cheat but surrounding each promise in another promise. So even if some fail, you still get back the ones that didn’t.

so if I take the code I had before

const fetch = require('node-fetch')

const astrosUrl = 'http://api.open-notify.org/astros.json'
const wikiUrl = 'https://en.wikipedia.org/api/rest_v1/page/summary'

function formatName(name) {
  return name.split(' ').join('_')
}

function getJSON (url) {
  return new Promise((resolve, reject) => {
    fetch(url)
      .then(response => response.json())
      .then(data => resolve(data))
      .catch(error => reject(error))
  })
}

function getProfiles(data) {
  const profiles = data.people.map(person => new Promise((resolve) => {
    getJSON(`${wikiUrl}/${formatName(person.name)}`)
      .then(data => resolve(data))
      .catch(error => resolve(error))
  }))

  return Promise.all(profiles)
}

function handleData(data) {
  data.forEach(item => {
    if (item instanceof Error) {
      return console.error(item.message)
    }

    console.log(item)
  })
}

getJSON(astrosUrl)
  .then(getProfiles)
  .then(handleData)
  .catch(error => console.error(error.message)) // probably should not trigger less astroUrl fails

kkoutoup · April 28, 2019, 12:40pm

Thanks very much for the suggestion. Things were much easier with fetch().
This is my approach in case anyone finds it useful. I also have to credit Treehouse as this is based on one of their courses.

My approach: https://bit.ly/2GIyJgK

ilenia · February 2, 2021, 11:09am

Promise.all() in node doesn't return value

Approach 1: Still using Promise.all

Approach 2: Call handleProfiles at the time of data return

Approach 1: Still using `Promise.all`

Approach 2: Call `handleProfiles` at the time of data return