Garlicvideos
Garlicvideos

Reputation: 29

For loop iterates before fs.readfile finishes

Expected behaviour: element.urls[j] == url, so fs.readFile is triggered. Eventually, the code gets to fs.writeFile and executes resolve(reply).

Current behaviour: Code runs as expected until fs.readFile. fs.readFile starts running synchronously, meaning the loop continues to iterate before the code block within has finished executing. This causes my loop to end prematurely, hence as fs.writeFile is complete and tries executing resolve(reply), it does nothing as the promise has already been rejected by the reject(reply) at the end which ends the loop once it reaches the end of the array.

Here is my code, please excuse the console.log()s. Was using them to debug:

    //Loop through monitor and access the "urls" key of each json
    let i = 0;
    for (let element of monitor) {
        //Checks if the url is in the array
        for (j = 0; j < element.urls.length; j++) {
            if (element.urls[j] == url) {
                console.log("yes the url is in the array");
                //The URL is in the array
                let item = monitor[i].product;
                //Removes the URL from the array
                monitor[i].urls.splice(j, 1);
                //Updates config.json with the new data
                console.log("removed the url from the arrauy");
                //Code splits into two branches here for some reason
                fs.readFile('./config.json', function (err, data) {
                    console.log("file is opened time to update");
                    var json = JSON.parse(data);
                    json.monitor = monitor;
                    fs.writeFile("./config.json", JSON.stringify(json), (err) => {
                        console.log("file updated");
                        let reply;
                        if (err) {
                            //There was an error trying to save the data
                            reply = [
                                {
                                    "name": "An error occured trying to remove the url from array",
                                    "value": `Error: ${err}`
                                }
                            ];
                            //Exit
                            reject(reply);
                            return;
                        }
                        //Success
                        reply = [
                            {
                                "name": "Successfully removed url from array",
                                "value": `I will now stop monitoring the url supplied`
                            },
                            {
                                "name": "Item name",
                                "value": item
                            },
                            {
                                "name": "URL supplied",
                                "value": url
                            }
                        ];
                        //Exit
                        resolve(reply);
                        return;
                    });
                });
            } else {
                console.log("monitor length");
                console.log(monitor.length-1);
                console.log("value of i");
                console.log(i);
                console.log("length of elements urls");
                console.log(element.urls.length-1);
                console.log("value of j");
                console.log(j);
                //Check if this is the final iteration
                if (i == monitor.length-1 && j == element.urls.length-1) {
                    console.log("ugh");
                    //This is the last iteration, the url supplied is not monitored by the bot
                    reply = [
                        {
                            "name": "The bot was never monitoring that URL",
                            "value": `The URL supplied isn't one that the bot currently monitors`
                        }
                    ];
                    //Exit
                    reject(reply);
                    return;
                }
            }
        }
        console.log("incrementing");
        //Increment i
        i++;
    };

Here is the console output so that the path of the code can be better visualised:

monitor length
1
value of i
0
length of elements urls
1
value of j
0
yes the url is in the array
removed the url from the arrauy
incrementing   <----- Shows that the loop is incrementing already
monitor length   <----- Start of the loop again
1
value of i
1
length of elements urls
0
value of j
0
ugh
?????   <----- Comes from outside the function, shows that the function this loop is in has already ended
file is opened time to update   <----- This comes from after fs.writeFile() is done opening the file
file updated   <----- fs.writeFile() is done saving the file to disk

As you can see, the first loop is broken up and fs.readFile() runs synchronously.

Any input is much appreciated. Been going at this for the past few hours now.

Upvotes: 1

Views: 1103

Answers (2)

endrcn
endrcn

Reputation: 319

You can use Sync methods in FileSystem module.

Reading: https://nodejs.org/api/fs.html#fs_fs_readfilesync_path_options -> fs.readFileSync(path[, options])

Writing: https://nodejs.org/api/fs.html#fs_fs_writefilesync_file_data_options -> fs.writeFileSync(file, data[, options])

Upvotes: 0

jfriend00
jfriend00

Reputation: 707308

fs.readFile() is non-blocking. That means your for loop continues to run while it starts all the fs.readFile() operations from your loop and they all are in-flight at the same time. Then, the for loop finishes and sometimes later your fs.readFile() operations finish and call their callback and they can finish in any order. So, at the time your for loop is done, none of the fs.readFile() completion callbacks will have been called yet and they can get called in any order. That should explain what you're seeing in your output.

To make them run in order, it is simplest to switch to using promises and the promise interface for readFile. Structurally, this would look like this:

// function needs to be async
async function someFunction() {
    for (...) {
        try {
            let data = await fs.promises.readFile(...);
            // do something with data
        } catch(e) {
            // handle error
        }
    }
    // all files have been processed here
}

This will process your files in order, pausing the for loop until each successive readFile() is done and running your asynchronous operations in sequence. And, at the end of your for loop, all the asynchronous operations will have completed.

This uses await to sequence the asynchronous operations in the for loop so the parent function has to be marked as async and it will return a promise that indicates when everything is done.

Upvotes: 2

Related Questions