2

I've got a function that reads date from files and stores them into an array. It is implemented asynchronously with async/await functionality.

The problem is that it is executing in the wrong order:

const util = require('util');
const fs = require('fs');
const path = require('path');

const readFile = util.promisify(fs.readFile);
const readDirectory = util.promisify(fs.readdir);

// Retrieve logs from logs files and store them into an array
const getAuditLogsData = async () => {
    const logsFolderPath = path.join(__dirname, '../', 'logs');
    const logData = [];

    try {
        const files = await readDirectory(logsFolderPath);
        // 1ST - OK
        console.log(files);
        files.forEach(async (file) => {

            const content = await readFile(logsFolderPath + '/' + file, 'utf-8');
            const logList = JSON.parse(content);

            logList.forEach((log) => {
                // 4TH - NOT OK
                console.log(1)
                logData.push(log);
            });

        });
        // 2ND - NOT OK
        console.log(2);
    } catch (error) {
        console.log(error);
    }

    // 3RD - NOT OK, EMPTY ARRAY (deta is 100% pushing in the forEach loop)
    console.log(logData);

    return logData;
};

module.exports = {
    getAuditLogsData
};

Is there something wrong with async/await promises?

UPDATE

I've updated the code to for-of loop, but it still didn't work:

try {
    const files = await readDirectory(logsFolderPath);
    // 1ST - OK
    console.log(files);

    for (const file of files) {
        const content = await readFile(logsFolderPath + '/' + file, 'utf-8');
        const logList = JSON.parse(content);

        logList.forEach((log) => {
            // 4TH - NOT OK
            console.log(1);
            // console.log(log);
            logData.push(log);
            // console.log(logData);
        });
    }

    // 2ND - NOT OK
    console.log(2);
} catch (error) {
    console.log(error);
}

Shall I change the fs methods to a synchronous one?

Can you tell me where is the mistake in this code?

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Karen
  • 1,249
  • 4
  • 23
  • 46
  • 3
    You pass an `async` callback to your `forEach` it will *definitely* be called after the `console.log(2)`. So, this is not "wrong", this is expected. You should probably be using `Promise.all`. – VLAZ Oct 08 '19 at 15:57
  • It's a great question. Look how many other people have had it! – Mulan Oct 08 '19 at 16:00
  • It is asynchronous nature of Node.js, it is running based on event loop queue not expected in order. – Sanjiv Oct 08 '19 at 20:51
  • The problem is the usage of forEach. forEach executes in async manner. If you want your code to execute synchronously get rid of forEach and use for as you are doing in your outer loop. – abhinav Oct 08 '19 at 21:11
  • Are you *really* still getting `1` after `2` in your update code? – Bergi Oct 08 '19 at 21:12

1 Answers1

1

Try await Promise.all(files.map(async _ => {...})), but keep in mind .all will reject on first failure.

Brandon Hill
  • 1,782
  • 14
  • 12