How to prevent an if condition from executing until readdir finishes in nodejs/express?

Question

app.get('/students/:record_id', function (req, res) {
  var found =false;
  var record_id = req.params.record_id;
  var dirname = "students/";


  filenames = fs.readdirSync(dirname);
  filenames.forEach(filename => {
    
    fs.readFile(dirname + filename, "utf-8", function (err, data ) {
      if (err) {
        onError(err);
        return;
      }
      if (data.includes(record_id)) {
        found = true;
        return res.status(200).send(data);
        
      }
    });
  });

  if(found == false){ 
    return res.status(500).send({ "message": "error - no student found" });
  }

});

I am using GET searching by a student name to retrieve a students file. If I remove the if condition it is successful. But if the student does not exist then it does not work. The if condition fixes this by making found true when it finds it in the file. The problem is that the if condition executes before the file is read. I tried using a callback but after a couple hours of research I can not figure out how to implement it in this since the readfile callback is being used to retrieve student info.

I tried using a promise but the promise is then only fulfilled if the student is found and I do not know how to implement it where it is fulfilled when it is not found.

`I tried using a promise` that's how I'd do it, reject on error, or reject if the forEach doesn't "complete" — Bravo, Mar 15 '22 at 23:31
How would I go about doing that? I am new to javascript so I am not used to thinking about functions running in asynch, @Bravo — Jarold Sabillon, Mar 15 '22 at 23:38
I was just about to post an answer and the question gets closed — Bravo, Mar 15 '22 at 23:38
So, here's a [pastebin](https://pastebin.com/SsLQRvc8) for you — Bravo, Mar 15 '22 at 23:39
No problems. that https://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call link is way over used as a duplicate and has such a wall of information in the answer it probably scares people rather than helps :p (IMNSHO) — Bravo, Mar 15 '22 at 23:44
By the way, I wouldn't use 500 status for student not found - unless a student not found really is an "internal server error" - which is what 500 indicates — Bravo, Mar 15 '22 at 23:46
looking closely, I've made a mistake ... it should be (I think) `import fs from 'fs/promises'` - but that would mean ALL `fs.` calls will return a Promise - so, not sure how that would effect code you didn't show (if you use `require` ... then you'll need to make the necessary adjustments) — Bravo, Mar 15 '22 at 23:49
Thanks @Keith - while the duplicate is valid, there's other aspects of the logic that I believe need to be addressed anyway — Bravo, Mar 15 '22 at 23:52
what you mean by require. I used const fs = require('fs'); const promise = require('promise'); @Bravo — Jarold Sabillon, Mar 15 '22 at 23:58

Bravo · Answer 1 · 2022-03-16T00:49:14.783

Since you've tried with Promises, I'll show you how to do it with Promises

Using async/await especially makes this really easy

Note: since I don't know how you are using fs the import/require only gets the promise versions of readdir and readFile, and uses it like readdir rather than fs.readdir - so other existing code won't break

const {readFile, readdir} = require('fs/promises');

// if you use import (modules) instead of require (commonjs)
// import { readFile, readdir } from 'fs/promises';

app.get('/students/:record_id', async function (req, res) {
    var record_id = req.params.record_id;
    var dirname = "students/";

    try {
        const filenames = await readdir(dirname);
        for (let filename of filenames) {
            const data = await readFile(dirname + filename, "utf-8");
            if (data.includes(record_id)) {
                return res.status(200).send(data);
            }
        }
        res.status(404).send({"message": "error - student not found"});
    } catch (err) {
        onError(err);
        res.status(500).send({"message": "internal server error"});
    }
});

Note: I wouldn't send a 500 (Internal Server Error) if a student is not found - I changed the logic to send 404 (not Found) instead - which is more appropriate

If you want pure promise (no async/await) - I believe the following is one way to do it, probably not the nicest code, but it should work (haven't tested though)

const {readFile, readdir} = require('fs/promises');

// if you use import (modules) instead of require (commonjs)
// import { readFile, readdir } from 'fs/promises';

app.get('/students/:record_id', function (req, res) {
    var record_id = req.params.record_id;
    var dirname = "students/";
    
    let p = Promise.resolve(false);
    for (let filename of filenames) {
        p = p
        .then(found => 
            found ? found : readFile(dirname + filename, "utf-8")
            .then(
                data => 
                    data.includes(record_id) ? data : false
            )
        );
    }
    .then(data => data ? res.status(200).send(data) : res.status(404).send({"message": "error - student not found"}))
    .catch(err => {
        onError(err);
        res.status(500).send({"message": "internal server error"});
    })
});

And, finally - without using Promises at all

app.get('/students/:record_id', function (req, res) {
    let found = false;
    const record_id = req.params.record_id;
    const dirname = "students/";

    const filenames = fs.readdirSync(dirname);
    const count = filenames.length;
    const checkFile = index => {
        if (index < count) {
            const filename = filenames[index];
            fs.readFile(dirname + filename, "utf-8", function (err, data) {
                if (err) {
                    onError(err);
                    res.status(500).send({"message": "internal server error"});
                } else if (data.includes(record_id)) {
                    res.status(200).send(data);
                } else {
                    checkFile(index + 1)
                }
            });
        } else {
            res.status(404).send({"message": "error - student not found"});
        }
    }
    checkFile(0);
});

So when testing it I get onError is not defined. I removed this but when testing a name that exists and one that does not, I only get internal server error — Jarold Sabillon, Mar 16 '22 at 00:03
I also get a warning that const filenames is not being used. — Jarold Sabillon, Mar 16 '22 at 00:05
typos - sorry - hang on - npot sure why `onError` is not defined ... it's not defined in YOUR code either - the missing `s` in `filename of filename` probably threw an error (it would) — Bravo, Mar 16 '22 at 00:09
Unfortunately neither of these methods are working even with the typo fixes — Jarold Sabillon, Mar 16 '22 at 00:30
For the first method, I see you have the await but where is the promise being made and fulfilled? — Jarold Sabillon, Mar 16 '22 at 00:32
`readdir` and `readFile` return a Promises - fullfilled on success, rejected on error - that's what the promise versions of `fs` do .... to be clear, `const {readFile, readdir} = require('fs/promises');` returns versions of fs methods that return promises - hence why there's `/promises` in the module name — Bravo, Mar 16 '22 at 00:36
@JaroldSabillon - FYI, I added code that doesn't use promises at all if that makes you more comfortable — Bravo, Mar 16 '22 at 00:49

How to prevent an if condition from executing until readdir finishes in nodejs/express?

1 Answers1