Node fetch loop too slow

Question

I have an API js file which I call with a POST method, passing in an array of objects which each contains a site url (about 26 objects or urls) as the body, and with the code below I loop through this array (sites) , check if each object url returns a json by adding to the url the "/items.json" , if so push the json content into another final array siteLists which I send back as response.

The problem is for just 26 urls, this API call takes more than 5 seconds to complete, am I doing it the wrong way or is it just the way fetch works in Node.js?

const sites content looks like:

[{label: "JonLabel", name: "Jon", url: "jonurl.com"},{...},{...}]

code is:

export default async (req, res) => {

    if (req.method === 'POST') {
        const body = JSON.parse(req.body)

        const sites = body.list  // this content shown above
        var siteLists = []
        
        if (sites?.length > 0){
            
            var b=0, idd=0
            while (b < sites.length){

                let url = sites?.[b]?.url

                if (url){

                    let jurl = `${url}/items.json`

                    try {
                        let fUrl = await fetch(jurl)
                        let siteData = await fUrl.json()
                
                        if (siteData){
                            let items = []
                            let label = sites?.[b]?.label || ""
                            let name = sites?.[b]?.name || ""
                            let base = siteData?.items
                        
                            if(base){
                                var c = 0
                                while (c < base.length){
                                    let img = base[c].images[0].url
                                    let titl = base[c].title

                                    let obj = {
                                        url: url,
                                        img: img,
                                        title: titl
                                    }
                                    items.push(obj)
                                    c++
                                }
                                let object = {
                                    id: idd,
                                    name: name,
                                    label: label,
                                    items: items
                                }
                                
                                siteLists.push(object)
                                idd++
                            }

                        }
                        
                    }catch(err){
                        //console.log(err)
                    }
                }
            
            b++
        }

        res.send({ sites: siteLists })
    }
res.end()
}

EDIT: (solution?) So it seems the code with promises as suggested below and marked as the solution works in the sense that is faster, the funny thing tho is it still takes more than 5 secs to load and still throws a Failed to load resource: the server responded with a status of 504 (Gateway Time-out) error, since Vercel, where the app is hosted passed to a max timeout of 5 secs for serverless functions, therefore never loading the content in the response. Locally, where I got no timeout limits is visibly faster to load, but it surprises me that such a query takes so long to complete where it should be a matter of ms.

user_6831341 · Answer

The biggest problem I see here is that you appear to be awaiting for one fetch to complete before you loop through to start the next fetch request, effectively running them serially. If you rewrote your script to run all of the simultaneously in parallel, you could push each request sequentially into a Promise.all and then process the results when they return.

Think of it like this-- if each request took a second to complete, and you have 26 requests, and you wait for one to complete before starting the next, it will take 26 seconds altogether. However, if you run them each all together, if they still each take only one second to complete the whole thing altogether will take just one second.

An example in psuedocode--

You want to change this:

const urls = ['url1', 'url2', 'url3'];

for (let url of urls) {
    const result = await fetch(url);
    process(result)
}

...into this:

const urls = ['url1', 'url2', 'url3'];

const requests = [];

for (let url of urls) {
    requests.push(fetch(url));
}

Promise.all(requests)
    .then(
        (results) => results.forEach(
            (result) => process(result)
        )
    );

user_12610347 · Answer

While await is a great sugar, sometimes it's better to stick with then

export default async (req, res) => {
    if (req.method === 'POST') {
        const body = JSON.parse(req.body)
        const sites = body.list  // this content shown above
        const siteListsPromises = []
        if (sites?.length > 0){
            var b=0
            while (b < sites.length){
                let url = sites?.[b]?.url
                if (url) {
                    let jurl = `${url}/items.json`
                    // #1
                    const promise = fetch(jurl)
                        // #2
                        .then(async (fUrl) => {
                            let siteData = await fUrl.json()
                            if (siteData){
                                ...
                                return {
                                    // #3
                                    id: -1,
                                    name: name,
                                    label: label,
                                    items: items
                                }
                            }
                        })
                        // #4
                        .catch(err => {
                            // console.log(err)
                        })
                    siteListsPromises.push(promise)
                }
                b++
            }
        }
        // #5
        const siteLists = (await Promise.all(siteListsPromises))
            // #6
            .filter(el => el !== undefined)
            // #7
            .map((el, i) => ({ id: i, ...el }))
        res.send({ sites: siteLists })
    }
    res.end()
}

Look for // #N comments in the snippet.

Don't await for requests to complete. Instead iterate over sites and send all requests at once
Chain json() and siteData processing after the fetch with then. And should your processing of siteData be more computational heavy it'd have even more sense to do so, instead of performing all of it only after all promises resolve.
If you (or someone on your team) have some troubles with understanding closures, don't bother setting the id of siteData elements in the cycle. I won't dive in this, but will address it further.
use .catch() instead of try{}catch(){}. Because without await it won't work.
await results of all requests with the Promise.all()
filter out those where siteData was falsy
finally set the id field.

Node fetch loop too slow

2 Answers