0

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.

Improve this question
3
  • 1
    I updated the code to show that fs is required Commented Aug 29, 2022 at 6:24
  • 1
    There's no need to use .then() in these cases. Commented Aug 29, 2022 at 6:27
  • 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. Commented Aug 29, 2022 at 6:43

1 Answer 1

Reset to default
2

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.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

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.
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.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.