Using multiple requests with promise

Question

I am trying to use request-promise module to check multiple web sites. If I use Promise.all as per design, promise returns with first reject. What is the proper way to execute multiple request tasks and wait all requests to finish whether they are fulfilled or rejected? I have come up with following two functions.

CheckSitesV1 returns exception due to one rejected promise. However CheckSitesV2 waits all promises to finish whether they are they are fulfilled or rejected. I will appreciate if you can comment whether the code I write makes sense. I am using NodeJS v7.9.0

const sitesArray = ['http://www.example.com','https://doesnt-really-exist.org','http://www.httpbin.org'];

async function CheckSitesV1() {
    let ps = [];
    for (let i = 0; i < sitesArray.length; i++) {
        let ops = {
            method: 'GET',
            uri:sitesArray[i],
        };
        const resp = await rp.get(ops);
        ps.push(resp);
    }
    return Promise.all(ps)
}

function CheckSitesV2() {
    let ps = [];
    for (let i = 0; i < sitesArray.length; i++) {
        let ops = {
            method: 'GET',
            uri:sitesArray[i],
        };
        ps.push(rp.get(ops));
    }
    return Promise.all(ps.map(p => p.catch(e => e)))
}

CheckSitesV1().then(function (result) {
    console.log(result);
}).catch(function (e) {
    console.log('Exception: ' + e);
});

CheckSitesV2().then(function (result) {
    console.log(result);
}).catch(function (e) {
    console.log('Exception: ' + e);
});

The "proper" way is the way that does exactly what you want to happen. `CheckSitesV1` does not resolve the promises in parallel whereas `CheckSitesV2` does. Which behavior do you want? — Felix Kling, May 31 '17 at 20:44
I want to check all sites whether I can reach it or not. `CheckSitesV1` returns rejected when one of the requests fails. I don't know if `CheckSitesV2` follows best practices. I didn't quite get async await in JS — Meanteacher, May 31 '17 at 20:46
But do you want the requests to be executed in parallel or in series? Because that's what the difference between these two solutions are. Also, I don't believe the first one actually doesn't do what you want. As soon as a promise is rejected, `CheckSitesV1` will throw: https://jsfiddle.net/gbw0c7x1/ — Felix Kling, May 31 '17 at 21:00
It doesn't matter for me to run them in parallel or in series. I would like to learn of course how to do both. First one doesn't request rest of URLs if it encounters an error. Because AFAIK, `Promise.all` returns with first rejection. — Meanteacher, May 31 '17 at 21:04
Sorry, forgot already that you were aware that the first one doesn't do what you want. However I'm confused now... what is your question really about? What is it that you are asking about `CheckSitesV1` if you already know that it doesn't do what you want it to do? FWIW, the reason why it doesn't work is not because of `Promise.all` (calling `Promise.all` is unnecessary here, you should be doing `return ps;`) but because `await` will throw an error if the promise that is awaited is rejected. If `CheckSitesV2` does what you want then why not just use it? Would you prefer async/await? — Felix Kling, May 31 '17 at 21:07
I am trying to understand how to make `CheckSitesV1` work by using async await just like `CheckSitesV2` So that I will have a flexibility to run the requests in parallel or in series. I am sorry for being so naive. This async await stuff didn't click for me yet :\ — Meanteacher, May 31 '17 at 21:10
`await` either returns the resolved value or throws the reject reason. So you have to catch the error: `try { ps.push(await rp.get(ops)); } catch (error) { ps.push(error); }`. [More about `await` in the MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await). — Felix Kling, May 31 '17 at 21:11

Karen Grigoryan · Accepted Answer · 2017-05-31T21:34:56.910

4

Your

function CheckSitesV2() {
    let ps = [];
    for (let i = 0; i < sitesArray.length; i++) {
        let ops = {
            method: 'GET',
            uri:sitesArray[i],
        };
        ps.push(rp.get(ops));
    }
    return Promise.all(ps.map(p => p.catch(e => e)))
}

is totally fine. I can only suggest refactor a bit for readability:

function CheckSitesV2() {
  let ps = sitesArray
    .map(site => rp.get({method: 'GET', uri: site}).catch(e => e));

  return Promise.all(ps);
}

Regarding async try this

async function CheckSitesV1() {
    let results = [];
    for (let i = 0; i < sitesArray.length; i++) {
        let opts = {
          method: "GET",
          uri: sitesArray[i]
        };
        const resp = await rp.get(opts).catch(e => e);
        results.push(resp);
    }
    return results;
}

CheckSitesV1().then(console.log)

edited May 31 '17 at 21:34

answered May 31 '17 at 21:11

Karen Grigoryan

5,234
2
21
35

With `async`/`await` most people likely would use `try`-`catch` though. Even when it's weird and more bloated than `.catch()` :-) – Bergi May 31 '17 at 22:19
@Bergi the whole purpose of `catch(e=>e)` is to suppress the error and pass it on to the next successful `then` as regular result. – Karen Grigoryan May 31 '17 at 22:32
Yes of course, but you could also do `try { results.push(await rp.get(opts)); } catch(e) { results.push(e); }` (which I presume would be more idiomatic, even if I don't like it) – Bergi May 31 '17 at 23:32

Using multiple requests with promise

1 Answers1