Express response is sent before the end of the loop

Question

There are tons of questions here on this topic and I've read through most of them but I cannot get my head around this problem. I am trying to add arrays containing URLs of files to the result of an SQL query. In NodeJS it works perfectly well but the response is sent too early, before any of the arrays of files have been added.

So far I've tried promises, async and synchronous fs.extra functions, npm async module, promisify. They were all very promising and likely to work but there is just something about this mysql query and first loop that I don't understand. This on especially where I tried both methods https://stackoverflow.com/a/70748789/12498040 and which I kind of what I have at the moment. The one with promises would "fail" the promises.all instead of waiting for them all

Anyway, here is the code : how would you make sure that res.json(result) is executed last? Thank you in advance :)

con.query(sql, function (err, result) {
        if (err) throw err;
        for (let i = 0; i < result.length; i++) {
            //these test arrays are being sent to the client
            result[i].test = ["aaa", "bbb", "ccc"]
            fs.existsSync(fichiers_observations + result[i].id_observation, (exists) => {
                if (exists) {
                    fs.readdirSync(fichiers_observations+result[i].id_observation, (err,files) => {
                        if (err) throw err;
                        result[i].files = [];
                        for (const file of files) {
                            //these URL are not being sent to the client
                            result[i].files.push('a URL/' + file)
                        }
                    });
                }
            })
        }
        res.json(result);
    })

score 0 · Answer 1 · answered Dec 01 '22 at 17:53

Right now you are using fx.existsSync() and fs.readdirSync() which are both synchronous operations. This means that the rest of the code will keep running after these functions are initiated.

Basically, the interpreter will start those operations, and then keep running the lines of code after them while they are running, because those functions indicate that the code is to be run 'synchronously', or, 'at the same time'. This means it will run the res.json() line right after it initiates the synchronous operations, which is obviously not the desired behavior.

If you want to wait for those functions to complete, you'll have to use async/await in combination with asynchronous versions of those functions. This will cause your code to run 'asynchronously', which means 'not at the same time', or 'one line after the other'.

Check out the documentation here:

fs.readdir() (what you want) vs fs.readdirSync() (what you have)
fs.stat() (what you probably want) vs fs.existsSync() (what you have)

I haven't tested your code, but if the implementation of those new functions is the same as the synchronous versions, then your new code could look something like this:

con.query(sql, async function (err, result) {
        if (err) throw err;
        for (let i = 0; i < result.length; i++) {
            //these test arrays are being sent to the client
            result[i].test = ["aaa", "bbb", "ccc"]
            try {
                const exists = await fs.stat(fichiers_observations + result[i].id_observation)
                if (exists) {
                    const files = await fs.readdir(fichiers_observations+result[i].id_observation)
                    result[i].files = [];
                    for (const file of files) {
                        //these URL are not being sent to the client
                        result[i].files.push('a URL/' + file)
                    }
                }
            } catch (err) {
                if (err) throw err;
            }
        }
        res.json(result);
})

Alternatively, you could find a way to do this using more callbacks, but using async/await is probably a lot cleaner.

Thank you, `await` was the way! fs.stat wouldn't return anything so I ended up keeping the await and using promisify :) async/await makes so much more sense now — Lenskha, Dec 02 '22 at 09:35

score 0 · Accepted Answer · answered Dec 02 '22 at 09:29

Here is the final code. await fs.stat would always return undefined so I had to revert to fs.exists and use await and promisify together. It looks simple now but there were so many ways it would not work, it was quite an experience.

const readdir = util.promisify(fs.readdir);
const exists = util.promisify(fs.exists);

con.query(sql, async function (err, result) {
    if (err) throw err;
    for (let i = 0; i < result.length; i++) {
        const it_exists = await exists(directory + result[i].id)
        if(it_exists){
            const files = await readdir(directory + result[i].id)
            result[i].files = [];
            for (const file of files) {
                result[i].files.push('a URL/' + file)
            }
        }
    }
    res.json(result)
})

Express response is sent before the end of the loop

2 Answers2