CODEWITHSUNDEEP
javascriptnode.jspromiserequest
acid
acid

Reputation: 73

NodeJS, request-promises are not waiting for each other

So I have broken my issue down a simple code snippet.

I want the otherRequestes to wait for my firstRequests, but somehow this is not working. the firstRequested is never waited for 

const rp = require('request-promise');

const firstRequest = () => {
  return rp('http://www.google.com')
    .then(function(htmlString) {
      console.log('in then of firstrequest');
    })
    .catch(function(err) {
      console.log('catch', err);
    });
}

laterRequest = (i) => {
  return rp('http://www.google.com')
    .then(function(htmlString) {
      console.log('in then' + i);
    })
    .catch(function(err) {
      console.log('catch', err);
    });

}

const requests = [];

for (let i = 0; i < 10; i += 1) {
  requests.push(laterRequest(i));
}

firstRequest().then(() => {
  Promise.all(requests).then(() => {
    console.log('all promises returned');
  });
});

So my desired output is "in then of firstrequest" after those there the output should be the laterRequests, I don't care for their order.

But when I run it currently my output is as follows the firstRequest ends up randomly somewhere in the output: 

in then0
in then5
in then2
in then3
in then1
in then8
in then4
in then6
in then9
in then7
in then of firstrequest
all promises returned

Upvotes: 0

Views: 644

Answers (2)

Aron
Aron

Reputation: 9258

Your firstRequest doesn't return a promise. You are already attaching a resolution handler to it by attaching a .then to it in the function.

You have a few different options for how to structure this code, but it looks like you want to have some things logged to the console when each promise succeeds or fails so you could do something like this:

const firstRequest = () => {
    return new Promise((resolve, reject) => { // actually returns a promise now
        rp('http://www.google.com')
            .then(function(htmlString) {
                console.log('in then of firstrequest');
                resolve(htmlString); // promise resolves (i.e. calles its `.then` function)
            })
            .catch(function(err) {
                console.log('catch', err);
                reject(err); // promise rejects (calles the `.catch`)
            });
    })
}

const laterRequest = (i) => {
    return new Promise((resolve, reject) => {
        rp('http://www.google.com')
            .then(function(htmlString) {
                console.log('in then' + i);
                resolve(htmlString);
            })
            .catch(function(err) {
                console.log('catch', err);
                reject(err);
            });
    })
}

const requests = [];

for (let i = 0; i < 10; i += 1) {
    requests.push(laterRequest(i));
}

firstRequest().then(() => {
    Promise.all(requests).then(() => {
        console.log('all promises returned');
    });
});

Shorter answer

You could also skip repeating thens and catches and just return rp's promise directly:

const firstRequest = () => rp('http://www.google.com');

const laterRequest = (i) => rp('http://www.google.com');


firstRequest().then(() => {

    const requests = [];
    for (let i = 0; i < 10; i += 1) {
        requests.push(laterRequest(i));
    }

    Promise.all(requests).then(() => {
        console.log('all promises returned');
    });
});

Upvotes: 1

MunkyJunky
MunkyJunky

Reputation: 379

When you do the for loop you're calling all the other promises before you've called the first one. Call your first promise, then in the .then() (when you know its finished) start calling the other promises.

firstRequest().then(() => {
  const requests = [];

  for (let i = 0; i < 10; i += 1) {
    requests.push(laterRequest(i));
  }

  return Promise.all(requests).then(() => {
    console.log('all promises returned');
  });
});

Upvotes: 1

Related Questions