How come my async function is not working in Node JS?

Question

I'm trying to make a REST api to hold json files for contacts, for some reason my endpoint responds before it should.

const fs = require("fs/promises");

app.get("/contacts", async (req, res) => {
    let dir_name = "./contacts";
    let contacts = [];

    
    
    await fs.readdir(dir_name).then(async (filenames) => {
        filenames.forEach(async (filename) => {
        await fs.readFile(`${dir_name}/${filename}`, "utf-8").then((jsonString) => {
            contacts.push(JSON.parse(jsonString));
            console.log(contacts);
        });
        });
    });

    console.log(contacts);
    res.send(contacts);
    
});

This code just results in [] being sent in response but the console.log under the json parse prints the result I'm expecting. I figured that I'm using the await keyword wrong but I'm stuck. Note: /contacts is a valid directory.

`forEach()` is not promise-aware. Do not use an `async` callback with `.forEach()` as the loop will not wait for your promises or you `await`. — jfriend00, Aug 29 '22 at 06:43

AKX · Answer 1 · 2022-08-29T06:40:09.760

You can use a regular for..of loop (which is "async aware" since it's not a function but a syntax construct) to read the files one by one while remaining async and not using blocking calls.

const fs = require("fs/promises");

app.get("/contacts", async (req, res) => {
    const dir_name = "./contacts";
    const contacts = [];
    const filenames = await fs.readdir(dir_name);
    for(const filename of filenames) {
        const content = await fs.readFile(`${dir_name}/${filename}`, "utf-8");
        contacts.push(JSON.parse(content));
    }
    console.log(contacts);
    res.send(contacts);
});

If you wanted to read them concurrently, you'd need to use Promise.all(filenames.map(...)):

const fs = require("fs/promises");

app.get("/contacts", async (req, res) => {
    const dir_name = "./contacts";
    const filenames = await fs.readdir(dir_name);
    const contacts = await Promise.all(filenames.map(async (filename) => {
        const content = await fs.readFile(`${dir_name}/${filename}`, "utf-8");
        return JSON.parse(jsonString);
    }));
    console.log(contacts);
    res.send(contacts);
});

That approach runs into the possibility of opening too many files at once, in which case you'd need to limit concurrency with e.g. p-queue or p-limit.

Not that it's directly related to the question, but I'd also mention to OP that it would probably be better to perform the file reading at the top level of the module and just `app.get("/contacts", (req, res) => { res.send(contacts); })` if they know that the JSON files will not be changed after the program starts. — Patrick Roberts, Aug 29 '22 at 06:43
Or maybe lazily with a memoized function, sure. But if the data can change on disk, then it naturally must be re-read every time. — AKX, Aug 29 '22 at 06:44

How come my async function is not working in Node JS?

1 Answers1