How to resolve multiple GET requests in a forEach using NodeJS

Question

I have a number of stock tickers in a JSON file that I want to use to send multiple GET requests to get the price of that stock. The problem I'm having is how to send them off in parallel and how to resolve them.

Here is the code I have tried:

const stocks = require('./stocks.json')
var request = require("request");

    var stockTickers = stocks.map(x => x.stockopediaTicker)
    stockTickers.forEach(ticker => {
        var promises = []
        var options = {
            method: 'GET',
            url: 'https://www.stockopedia.com/ajax/get_prices/' + ticker + '/'
        };
        let todaysQuote = new Promise(function (res, rej) {
            request(options, function (error, response, body) {
                rej(error)
                res(body)
            });
        })
        promises.push(todaysQuote)
    });

    Promise.all(promises)
        .then((results) => {
            console.log(results)
        }).catch(err => console.log(err))

var promises = [] is defined in a scope that Promise.all doesn't have access to. Try defining promises outside the forEach — sissonb
– sissonb, Commented Jun 14, 2019 at 18:10

jfriend00 · Accepted Answer · 2019-06-14 22:26:22Z

You were on the right path with your code.

You just need to only call rej(err) if there is actually an error
You shouldn't be attempting to share the same options object with all your requests (that might or might not cause problems)
You need to declare promises in a higher scope where it can be used with Promise.all(promises):

Here's what it would look like after fixing up those issues:

const stocks = require('./stocks.json')
const request = require("request");

let promises = [];
let stockTickers = stocks.map(x => x.stockopediaTicker);

stockTickers.forEach(ticker => {
    let todaysQuote = new Promise(function (res, rej) {
        let options = {
            method: 'GET',
            url: 'https://www.stockopedia.com/ajax/get_prices/' + ticker + '/'
        };
        request(options, function (error, response, body) {
            if (error) {
                rej(error);
            } else {
                res(body);
            }
        });
    })
    promises.push(todaysQuote)
});

Promise.all(promises)
    .then((results) => {
        console.log(results);
    }).catch(err => console.log(err))

The request-promise module is simpler to use as it already includes a promise wrapper and it's probably easier to use .map() to accumulate your array of promises.

const stocks = require('./stocks.json')
const rp = require("request-promise");

Promise.all(stocks.map(x => {
    let options = {
        method: 'GET',
        url: 'https://www.stockopedia.com/ajax/get_prices/' + x.stockopediaTicker + '/'
    };
    return rp(options);
})).then(results => {
    console.log(results);
}).catch(err => {
    console.log(err);
});

MattB · Accepted Answer · 2019-06-14 17:53:26Z

0

You should check if there is an error and only rej if there is an error:

let todaysQuote = new Promise(function (res, rej) {
        request(options, function (error, response, body) {
            if(error) {
                return rej(error)
            }

            res(body)
        });
    })

Right now, you're "rejecting" every response.

answered Jun 14, 2019 at 17:53

MattB

1,1048 silver badges15 bronze badges

1 Comment

Jamie Over a year ago

I was under the impression that a promise was only rejected if request raised an error?

Christian Colón · Accepted Answer · 2019-06-14 18:50:11Z

I'm not sure about "request" but using "request-promise-native" you can simplify some things like this.

const stocks = require('./stocks.json');
const request = require('request-promise-native');

const parseStocks = (stocks)=>Promise.all(stocks.map(requestQuote));

const requestQuote = ({stockopediaTicker})=>{
    const options = {
        method: 'GET',
        url: `https://www.stockopedia.com/ajax/get_prices/${stockopediaTicker}/`
    };
    return request(options)
}

parseStocks(stocks).then(console.log).catch(console.log)

Normally I refrain from suggesting dependency changes in an answer, but in this case, 'request-promise-native' is suggested by the 'request' docs. If you plan on using promises you may want to switch. It's usually best practice to avoid combining callbacks and promise chains together.

Collectives™ on Stack Overflow

How to resolve multiple GET requests in a forEach using NodeJS

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related