Reputation: 508
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 error:
Failed to load resource: the server responded with a status of 504 (Gateway Time-out)
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.
Upvotes: 3
Views: 2381
Reputation: 13823
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.
await
for requests to complete. Instead iterate over sites
and send all requests at oncejson()
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.id
of siteData
elements in the cycle. I won't dive in this, but will address it further..catch()
instead of try{}catch(){}
. Because without await
it won't work.await
results of all requests with the Promise.all()
siteData
was falsyid
field.Upvotes: 1
Reputation: 13623
The biggest problem I see here is that you appear to be await
ing 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)
)
);
Upvotes: 3