Script only prints last file in array instead of all files

Question

I'm reading about Promises in JavaScript The Definitive Guide by Flannagan 7ed. In the book there is a script which shows how to build a Promise chain dynamically for an arbitrary number of URLs. The script is as follows:

function fetchSequentially(urls) {
    
    // We'll store the URL bodies here as we fetch them
    const bodies = [];

    // Here's a Promise-returning function that fetches one body
    function fetchOne(url) {
        
        return fetch(url)
                .then(response => response.text())
                .then(body => {
                    // We save the body to the array, and we're purposely
                    // omitting a return value here (returning undefined)
                    bodies.push(body);
                });
    }

    // Start with a Promise that will fulfill right away (with value undefined)
    let p = Promise.resolve(undefined);

    // Now loop through the desired URLs, building a Promise chain
    // of arbitrary length, fetching one URL at each stage of the chain
    for (url of urls) {
        p = p.then(() => fetchOne(url));
    }

    // When the last Promise in that chain is fulfilled, then the
    // bodies array is ready. So let's return a Promise for that
    // bodies array. Note that we don't include any error handlers:
    // we want to allow errors to propagate to the caller.
    
    return p.then(() => bodies);
}

//The script was run as below
//I added the line below to declare the urls array

let urls = ['/data.txt', '/readme.txt', '/textfile.txt'];

//the line below is from the book
fetchSequentially(urls)
    .then(bodies => {
        console.log(bodies)
    })
    .catch(e => console.error(e));

I added the let urls line to run the script to fetch 3 text files on my PC.

When the script runs it seems to only fetch the last file textfile.txt, and it prints out the contents of the third file 3 times in the console. I thought the script would retrieve the contents of all 3 files, add them to the bodies array, and then log the contents of all 3 files to console.

Can anyone spot why this isn't working?

It's not clear why this isn't working - perhaps show us your code that uses this and logs the results (there is no `console.log` in the code you've shown us) so we can see if you're doing something wrong. PS I don't know what problem this code is solving, you should fetch in parallel (with `Promise.all`), not sequentially, if you have a bunch of URLs and none depend on the result of another (which is unlikely). — Robin Zigmond, Apr 15 '22 at 14:49
Any chance the three files actually have the same content? I know that's 100% something that would happen to me. — Aioros, Apr 15 '22 at 14:50
The console.log is called in the last then function ie. then( bodies => {console.log(bodies)}) — John D, Apr 15 '22 at 14:59

Steve · Accepted Answer · 2022-04-15T15:01:09.997

It looks like this is the section that's causing problems:

   for(url of urls) {
       p = p.then(() => fetchOne(url));
   }

Here you're creating a global variable url, and since it's running asynchronously, fetchOne(url) is using the last instance of it.

Instead you can do something like:

   for(let url of urls) {
       p = p.then(() => fetchOne(url));
   }

This creates a local instance of url for each iteration.

This sort of style of programming of iterating through arrays asynchronously can introduce subtle errors like this one, so I'd recommend a style that unambiguously creates a new instance per iteration. Something like:

urls.forEach(function (url) {
   p = p.then(() => fetchOne(url));
});

Though for this sort of thing with multiple promises, you might just want to do a .map with Promise.all:

return Promise.all(urls.map(fetchOne)); // instead of promise chaining with p

Thanks. Declaring 'url' with let in the for loop has worked – John D Apr 15 '22 at 14:58 — John D, Apr 15 '22 at 14:58

Script only prints last file in array instead of all files

1 Answers1