0

I've got the following function, which doesn't execute in the order I'd like it to.

The console.log within the callback actually gets fired before the loop within the function itself even executes. So to sum it up I'd like the function "getFiles" to fully finish before the next one "createDatabase" starts to execute.

Thanks for your help. I'm already going nuts about this issue :(

const fs = require("fs")
let data = []

const getFiles = () => {
  fs.readdir("./database/test/", (err, files) => {
    files.forEach(file => {
      require("../database/test/" + file)
      data.push(Number(file.split(".")[0]))
      console.log(data)
    })
  })
}

const createDatabase = () => {
  data.sort((a, b) => {
    return a-b
  })
  console.log(data)
  // Do some other stuff
}


const run = async () => {
  await getFiles
  await createDatabase()
  process.exitCode = 0
}

run()

Architecd
  • 63
  • 1
  • 8
  • I'm not sure how the await keyword works exactly, but why do you have `getFiles` but then `createDatabase()` (the parentheses), aren't they both functions? – Matthias Dec 28 '19 at 20:05
  • 1
    `await getFiles` 1. this will not execute the function 2. even if you execute the function, it doesn't return a Promise, so it's useless to await it. – VLAZ Dec 28 '19 at 20:06
  • The missing parenthesis was a typo. Sorry for that. – Architecd Dec 28 '19 at 20:21

4 Answers4

2

This would be fairly straightforward to code with the newer fs.promises interface so you can await the readdir() output:

const fsp = require('fs').promises;
const path = require('path');

const async getFiles = () => {
  const root = "./database/test/";
  let files = await fsp.readdir(root);
  return files.map(file => {
      require(path.join(root, file));
      return Number(file.split(".")[0]);
  });
}

const createDatabase = (data) => {
  data.sort((a, b) => {
    return a-b
  })
  console.log(data)
  // Do some other stuff
}

const run = async () => {
  let data = await getFiles();
  createDatabase(data);
  process.exitCode = 0
}

run().then(() => {
    console.log("all done");
}).catch(err => {
    console.log(err);
});

Summary of Changes:

  1. Use fs.promises interface so we can await the results of readdir().
  2. Change getFiles() to be async so we can use await and so it returns a promise that is resolved when all the asynchronous work is done.
  3. Move data into the functions (resolved from getFiles() and passed to createDatabase()) rather than being a top level variable that is shared by multiple functions.
  4. Track completion and errors from calling run()
  5. Actually call getFiles() with parens after it in run().
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Awesome, thanks a lot :) That was just what I needed to get back on track :) [I'd love to upvote your answer but am not allowed to do so yet :(] – Architecd Dec 28 '19 at 20:48
  • Hey @jfriend00, your code works just fine. Unfortunately I'm now stuck setting a synchronous .map call into the place of "Do some other stuff" though. I want it to execute fully before the .then function at the end of the script starts to run. Already tried it in different ways but the console.log("all done") always executes before the .map function is done :( Any ideas? – Architecd Dec 28 '19 at 23:17
  • @Architecd - it sounds like you an async operation inside the .map() and .map() is not async aware. I'd suggest posting a new question with your actual code. – jfriend00 Dec 28 '19 at 23:19
1

Following code snippet will first completely run getFiles function and then run the createDatabase function. For using async/await, a function needs to return a promise(Read about promises here).

const fs = require("fs")
let data = []

const getFiles = () => {
  return new Promise((resolve, reject) => {
    fs.readdir("./database/test/", (err, files) => {
      if (err) {
        reject(err);
      }
      files.forEach(file => {
        require("../database/test/" + file)
        data.push(Number(file.split(".")[0]))
        console.log(data)
      })
      resolve(true);
    })
  })
}

const createDatabase = () => {
  data.sort((a, b) => {
    return a-b
  })
  console.log(data)
  // Do some other stuff
}


const run = async () => {
  await getFiles();
  await createDatabase();
  process.exitCode = 0;
}

run()

Since the createDatabase function is not returning any promise right now, you can remove await in front of it. Await is used only if a promise is returned. You can read about async/await here.

Satvik Daga
  • 156
  • 4
  • Thanks a lot :) That works like a charm as well and explains clearly how I can alter my code to make it work in future. – Architecd Dec 28 '19 at 20:57
  • @Architecd Glad to be able to help. Also one update, error handling with async/await is done using try catch blocks. – Satvik Daga Dec 28 '19 at 21:09
-1

The problem is related to using a forEach loop for asynchronous code. This answer has more on the issue:

Using async/await with a forEach loop

ElllGeeEmm
  • 41
  • 3
  • Hi ElllGeeEmm, welcome to Stack Overflow! Please quote the relevant part of the answer you linked to so that this answer can stand on its own. – stephenwade Dec 28 '19 at 21:29
-1

I am not really sure what exactly you are trying to do but looks like the problem is in your run function. You are not calling getFiles function inside run. You care just mentioning the function name. Also needs to wrap the getFiles in promise it should be

const run = async () => {
  await getFiles();
  await createDatabase();
  process.exitCode = 0
}
Ashish Modi
  • 7,529
  • 2
  • 20
  • 35
  • 1
    `getFiles()` doesn't return a promise that is linked to it's completion so doing `await getFiles()` doesn't wait for it to be done. – jfriend00 Dec 28 '19 at 20:15
  • This would still not ensure all operations in `getFiles` would be executed before `createDatabase` is executed. – VLAZ Dec 28 '19 at 20:15
  • What would have to be changed in order to achieve "getFiles" being executed fully before "createDatabase" gets called? – Architecd Dec 28 '19 at 20:21
  • my bad I looked at the code written by someone in one of the answer instead of original post which had getFiles wrapped in Promise. – Ashish Modi Dec 28 '19 at 20:29