Using async await properly in node js

To overcome callback hell in javascript, I'm trying to use async await from legacy code written in SQLServer procedure. But I'm not sure my code might be write properly.

My first confusing point is when async function returns, should it return resolve() as boolean, or just return reject and handle with try-catch?

Here is my code snippets. Please correct me to right direction.

apiRoutes.js

app.route('/api/dansok/cancelDansok')
    .post(dansokCancelHandler.cancelDansok);

dansokCancelController.js

const sequelize = models.Sequelize;
const jwt = require('jsonwebtoken');

async function jwtAccessAuthCheck(accessToken) {
  if (!accessToken) {
    return Promise.reject('Empty access token');
  }

  jwt.verify(accessToken,"dipa",function(err){
    if(err) {
      return Promise.reject('TokenExpiredError.');
    } else {
      return Promise.resolve();
    }
  });
}

async function checkFeeHist(dansokSeqNo) {
  let feeHist = await models.FeeHist.findOne({  
                  where: { DansokSeqNo: dansokSeqNo}
                });
  return !!feeHist;
}

async function getNextDansokHistSerialNo(dansokSeqNo) {
  ....
}

async function getDansokFee(dansokSeqNo) {
  ....
}

async function doCancel(dansokSeqNo) {
  try {
    if (await !checkFeeHist(dansokSeqNo)) {
      log.error("doCancel() invalid dansokSeqNo for cancel, ", dansokSeqNo);
      return;
    }
    let nextDansokSerialNo =  await getNextDansokHistSerialNo(dansokSeqNo);
    await insertNewDansokHist(dansokSeqNo, nextDansokSerialNo);
    await updateDansokHist(dansokSeqNo);
    await updateVBankList(dansokSeqNo, danokFee.VBankSeqNo);
    await getVBankList(dansokSeqNo);
  } catch (e) {
    log.error("doCancel() exception:", e);
  }
}

exports.cancelDansok = function (req, res) {
  res.setHeader("Content-Type", "application/json; charset=utf-8");
  const dansokSeqNo = req.body.DANSOKSEQNO;
  const discKindCode = req.body.HISTKIND;
  const worker = req.body.PROCWORKER;
  const workerIp = req.body.CREATEIP;
  const accessToken = req.headers.accesstoken;

  //check input parameter
  if (!dansokSeqNo || !discKindCode || !worker || !workerIp) {
    let e = {status:400, message:'params are empty.'};
    return res.status(e.status).json(e);
  } 

  try {
    jwtAccessAuthCheck(accessToken)
    .then(() => {
      log.info("jwt success");
      doCancel(dansokSeqNo).then(() => {
        log.info("cancelDansok() finish");
        res.status(200).json({ message: 'cancelDansok success.' });
      });
    });
  } catch(e) {
    return res.status(e.status).json(e);
  }
};

Solution

You'll need to rewrite jwtAccessAuthCheck(accessToken) so that it keeps track of the outcome of its nested tasks. In the code you've written:

// Code that needs fixes!
async function jwtAccessAuthCheck(accessToken) {

  // This part is fine. We are in the main async flow.
  if (!accessToken) {
    return Promise.reject('Empty access token');
  }

  // This needs to be rewritten, as the async function itself doesn't know anything about
  // the outcome of `jwt.verify`... 
  jwt.verify(accessToken,"dipa",function(err){
    if(err) {
      // This is wrapped in a `function(err)` callback, so the return value is irrelevant
      // to the async function itself
      return Promise.reject('TokenExpiredError.');
    } else {
      // Same problem here.
      return Promise.resolve();
    }
  });

  // Since the main async scope didn't handle anything related to `jwt.verify`, the content
  // below will print even before `jwt.verify()` completes! And the async call will be
  // considered complete right away.
  console.log('Completed before jwt.verify() outcome');

}

A better rewrite would be:

// Fixed code. The outcome of `jwt.verify` is explicitly delegated back to a new Promise's
// `resolve` and `reject` handlers, Promise which we await for.
async function jwtAccessAuthCheck(accessToken) {
  await new Promise((resolve, reject) => {

    if (!accessToken) {
      reject('Empty access token');
      return;
    }

    jwt.verify(accessToken,"dipa",function(err){
      if(err) {
        reject('TokenExpiredError.');
      } else {
        resolve();
      }
    });

  });

  // We won't consider this async call done until the Promise above completes.
  console.log('Completed');

}

An alternate signature that would also work in this specific use case:

// Also works this way without the `async` type:
function jwtAccessAuthCheck(accessToken) {
  return new Promise((resolve, reject) => {
    ...
  });
}

Regarding your cancelDansok(req, res) middleware, since jwtAccessAuthCheck is guaranteed to return a Promise (you made it an async function), you'll also need to handle its returned Promise directly. No try / catch can handle the outcome of this asynchronous task.

exports.cancelDansok = function (req, res) {

  ...

  jwtAccessAuthCheck(accessToken)
    .then(() => {
      log.info("jwt success");
      return doCancel(dansokSeqNo);
    })
    .then(() => {
      log.info("cancelDansok() finish");
      res.status(200).json({ message: 'cancelDansok success.' });
    })
    .catch(e => {
      res.status(e.status).json(e);
    });

};

I strongly suggest reading a few Promise-related articles to get the hang of it. They're very handy and powerful, but also bring a little pain when mixed with other JS patterns (async callbacks, try / catch...).