Refactoring promise function to es6 format, await doesn't actually await

Question

So I'm just revisiting some old code to tidy up and modernize some syntax in my Partum-React project. But when I reformat this specific older traditional function() return new Promise... to an async () => ...return... type function, it doesn't seem to await for the function to be finished. The answers I found don't seem to solve my problem, since as far as I can tell, my async function is written correctly and should work as expected. I also already reformatted some older function/promise functions into async functions without an issue. So I'm pretty stumped on why this specific function doesn't seem to get properly wrapped in a promise.

The old function which works as expected, program properly awaits for the function to complete before continuing execution (found in helper.js in the /src folder):

function shell(command, log = false, cb = null) {
  return new Promise((resolve, reject) => {
    exec(command, (err, stdout, stderr) => {
      if (err) {
        reject(err);
      } else {
        if (cb) cb(stdout);
        if (log) process.stdout.write(`\n${stdout}\n\n`);
        resolve({
          stdout,
          stderr,
        });
      }
    });
  });
}

Rewritten function, doesn't seem to properly await when called with await. consecutive shell calls are run synchronously causing git to error out. I'm sure it's a small mistake on my part, but I just can't seem to see it. As far as I know, this function should be properly wrapped in a promise and function as such:

const shell = async (command, log = false, cb = null) => {
  exec(command, (err, stdout, stderr) => {
    if (err) throw err;
    if (cb) cb(stdout);
    if (log) process.stdout.write(`\n${stdout}\n\n`);
    return {
      stdout,
      stderr,
    };
  });
};

This is where the function is being called (found in initializer.js in the /src folder) (I know, this also needs some major cleaning up):

finalize() {
    return new Promise((resolve, reject) => {
      process.stdout.write('finalizing project\n');
      copy(this.tempPath, this.destination, (err) => {
        if (err) {
          reject(err);
        } else {
          process.stdout.write('cleaning up temporary files\n');
          rmdir(this.tempPath, async (err) => { // eslint-disable-line
            if (err) {
              reject(err);
            } else {
              try {
                await shell(`mkdir ${path.join(this.destination, this.options.componentPath)}`);

                if (this.options.redux) {
                  await shell(`mkdir ${path.join(this.destination, this.options.reduxPath, this.options.actionPath)}`);
                }

                await shell(`git -C ${this.destination} init`);
                await shell(`git -C ${this.destination} add .`);
                await shell(`git -C ${this.destination} commit -m 'Initialized with Partum-React'`);

                process.stdout.write(
                  `\nrunning npm install inside ${this.destination}\n`,
                );
                await npmInstall(this.destination, this.silent);

                resolve(true);

              } catch (err) { // eslint-disable-line
                reject(err);
              }
            }
          });
        }
      });
    });
  }
};

console logging shell(....) without await logs Promise {undefined}

console logging await shell(....) with await logs {undefined}

Thank you in advance for any help on this!

No, the rewritten function won't be properly awaited as it does not return a promise. The 'old' function is a correct version if you want to use it with `async/await` syntax. — Sergii Vorobei, Aug 28 '19 at 20:18
If in the old syntax you had to use `new Promise`, then moving to the newer syntax does not mean you can drop that part. Also, an `async` function without the `await` keyword is most often a sign that something is wrong. In your case there is no good reason to convert your function to the `async` syntax. The latter is more interesting for when you have a chain of `then` calls. — trincot, Aug 28 '19 at 20:36
I was under the impression that an async function will always return a value as a promise without needing to explicitly construct a new promise return value. Is this not the case? — Highwalker, Aug 28 '19 at 23:51

Bergi · Accepted Answer · 2019-08-29T20:12:01.510

No, your async function is not written correctly. You cannot replace the new Promise constructor by async/await syntax. If something provides only a callback API, you need to promisify it before being able to use await on it.

You should be doing

function shell(command) {
  return new Promise((resolve, reject) => {
    exec(command, (err, stdout, stderr) => {
      if (err) {
        reject(err);
      } else {
        resolve({stdout, stderr});
      }
    });
  });
}
function copy(source, destination) {
  return new Promise((resolve, reject) => {
    …
  });
}
function rmdir(path) {
  return new Promise((resolve, reject) => {
    …
  });
}

and then you can write your finalise function using async/await:

async finalize() {
  process.stdout.write('finalizing project\n');
  await copy(this.tempPath, this.destination);
  process.stdout.write('cleaning up temporary files\n');
  await rmdir(this.tempPath);
  await shell(`mkdir ${path.join(this.destination, this.options.componentPath)}`);
  if (this.options.redux) {
    await shell(`mkdir ${path.join(this.destination, this.options.reduxPath, this.options.actionPath)}`);
  }
  await shell(`git -C ${this.destination} init`);
  await shell(`git -C ${this.destination} add .`);
  await shell(`git -C ${this.destination} commit -m 'Initialized with Partum-React'`);
  process.stdout.write(`\nrunning npm install inside ${this.destination}\n`);
  await npmInstall(this.destination, this.silent);
  return true;
}

score -1 · Answer 2 · answered Aug 29 '19 at 17:53

const shell = async (command, log = false, cb = null) => {
  /*problem lies here, because most probably this is a async function and 
  therefore undefined is returned from this function as resolved value*/

  exec(command, (err, stdout, stderr) => {
    if (err) throw err;
    if (cb) cb(stdout);
    if (log) process.stdout.write(`\n${stdout}\n\n`);
    return {
      stdout,
      stderr,
    };
  });
};

see this question

Refactoring promise function to es6 format, await doesn't actually await

2 Answers2