CODEWITHSUNDEEP
node.jsasynchronousasync-awaitcommand-line-interfacewaterfall
adhamfarrag
adhamfarrag

Reputation: 47

Async/Await doesn't await

Building a node.js CLI application. Users should choose some tasks to run and based on that, tasks should work and then spinners (using ora package) should show success and stop spin.

The issue here is spinner succeed while tasks are still going on. Which means it doesn't wait.

Tried using typical Async/Await as to have an async function and await each function under condition. Didn't work.

Tried using promise.all. Didn't work.

Tried using waterfall. Same.

Here's the code of the task runner, I create an array of functions and pass it to waterfall (Async-waterfall package) or promise.all() method.

const runner = async () => {
    let tasks = [];

    spinner.start('Running tasks');

    if (syncOptions.includes('taskOne')) {
        tasks.push(taskOne);
    }

    if (syncOptions.includes('taskTwo')) {
        tasks.push(taskTwo);
    }

    if (syncOptions.includes('taskThree')) {
        tasks.push(taskThree);
    }

    if (syncOptions.includes('taskFour')) {
        tasks.push(taskFour);
    }

    // Option One
    waterfall(tasks, () => {
        spinner.succeed('Done');
    });

    // Option Two
    Promise.all(tasks).then(() => {
        spinner.succeed('Done');
    });
};

Here's an example of one of the functions:

const os = require('os');
const fs = require('fs');

const homedir = os.homedir();
const outputDir = `${homedir}/output`;
const file = `${homedir}/.file`;
const targetFile = `${outputDir}/.file`;

module.exports = async () => {
    await fs.writeFileSync(targetFile, fs.readFileSync(file));
};

I tried searching concepts. Talked to the best 5 people I know who can write JS properly. No clue.. What am I doing wrong ?

Upvotes: 0

Views: 1456

Answers (1)

jfriend00
jfriend00

Reputation: 707218

You don't show us all your code, but the first warning sign is that it doesn't appear you are actually running taskOne(), taskTwo(), etc...

You are pushing what look like functions into an array with code like:

tasks.push(taskFour);

And, then attempting to do:

Promise.all(tasks).then(...)

That won't do anything useful because the tasks themselves are never executed. To use Promise.all(), you need to pass it an array of promises, not an array of functions.

So, you would use:

tasks.push(taskFour());

and then:

Promise.all(tasks).then(...);

And, all this assumes that taskOne(), taskTwo(), etc... are function that return a promise that resolves/rejects when their asynchronous operation is complete.

In addition, you also need to either await Promise.all(...) or return Promise.all() so that the caller will be able to know when they are all done. Since this is the last line of your function, I'd generally just use return Promise.all(...) and this will let the caller get the resolved results from all the tasks (if those are relevant).

Also, this doesn't make much sense:

module.exports = async () => {
    await fs.writeFileSync(targetFile, fs.readFileSync(file));
};

You're using two synchronous file operations. They are not asynchronous and do not use promises so there's no reason to put them in an async function or to use await with them. You're mixing two models incorrectly. If you want them to be synchronous, then you can just do this:

module.exports = () => {
    fs.writeFileSync(targetFile, fs.readFileSync(file));
};

If you want them to be asynchronous and return a promise, then you can do this:

module.exports = async () => {
    return fs.promises.writeFile(targetFile, await fs.promises.readFile(file));
};

Your implementation was attempting to be half and half. Pick one architecture or the other (synchronous or asynchronous) and be consistent in the implementation.

FYI, the fs module now has multiple versions of fs.copyFile() so you could also use that and let it do the copying for you. If this file was large, copyFile() would likely use less memory in doing so.

As for your use of waterfall(), it is probably not necessary here and waterfall uses a very different calling model than Promise.all() so you certainly can't use the same model with Promise.all() as you do with waterfall(). Also, waterfall() runs your functions in sequence (one after the other) and you pass it an array of functions that have their own calling convention.

So, assuming that taskOne, taskTwo, etc... are functions that return a promise that resolve/reject when their asynchronous operations are done, then you would do this:

const runner = () => {
    let tasks = [];

    spinner.start('Running tasks');

    if (syncOptions.includes('taskOne')) {
        tasks.push(taskOne());
    }

    if (syncOptions.includes('taskTwo')) {
        tasks.push(taskTwo());
    }

    if (syncOptions.includes('taskThree')) {
        tasks.push(taskThree());
    }

    if (syncOptions.includes('taskFour')) {
        tasks.push(taskFour());
    }

    return Promise.all(tasks).then(() => {
        spinner.succeed('Done');
    });
};

This would run the tasks in parallel.

If you want to run the tasks in sequence (one after the other), then you would do this:

const runner = async () => {

    spinner.start('Running tasks');

    if (syncOptions.includes('taskOne')) {
        await taskOne();
    }

    if (syncOptions.includes('taskTwo')) {
        await taskTwo();
    }

    if (syncOptions.includes('taskThree')) {
        await taskThree();
    }

    if (syncOptions.includes('taskFour')) {
        await taskFour();
    }

    spinner.succeed('Done');
};

Upvotes: 2

Related Questions