How do I return a valid response from an asynchronous method using Express?

Question

Still getting the hang of Node's non-blocking nature. The following code executes as intended. However, I am wondering if there is a better approach to accomplish the task.

There are 3 params being supplied to the route (zipcode, type, rad). From there I use the NPM Zipcode package to return an array of zipcodes within the provided rad.

I am then using a for loop on the zips array within an async function and awaiting the response of a function which performs a MySQL query and returns a promise. Then returning an array of user objects.

My uncertainty is whether I am sending the response correctly or if there is a more efficient way to write this code.

Thanks.

router.get('/:zipcode/:type/:rad', function (req, res) {

  const rad = req.params.rad;
  const zip = req.params.zipcode;
  let zips = zipcodes.radius(zip, rad);
  zips.push(zip);

  let type;
  if(req.params.type === 'bartenders') {
    type = 0;
  } else {
    type = 1;
  }

  const params = {
    'type': type,
    'zips': zips
  };


  function userGroup(type, zip) {
    return new Promise(resolve => {
      connection.query(`SELECT * FROM bt9.users WHERE zip = ${zip} AND type = ${type} AND display = 1`, function (err, result) {
        if(err) throw err;
        resolve(result);
      });
    });
  }


  async function getUsers(params) {
    let userList = [];
    for (i = 0; i < params.zips.length; i++) {
      const users = await userGroup(params.type, params.zips[i]);
      for (u = 0; u < users.length; u++) {
        userList.push(users[u]);
      }
    }
    return userList;
  }


  function sendUsers(callback) {
    getUsers(params).then( res => {
      callback(null, res)
    })
  }


  sendUsers(function(err, result) {
    if(err) throw err;
    res.send(result)
  })


});

Answer 1

Express 5 will automatically handle async errors correctly

https://expressjs.com/en/guide/error-handling.html currently says it clearly:

Starting with Express 5, route handlers and middleware that return a Promise will call next(value) automatically when they reject or throw an error. For example:

app.get('/user/:id', async function (req, res, next) {
 var user = await getUserById(req.params.id)
 res.send(user)
})

If getUserById throws an error or rejects, next will be called with either the thrown error or the rejected value. If no rejected value is provided, next will be called with a default Error object provided by the Express router.

I have shown that in an experiment at: Passing in Async functions to Node.js Express.js router

This means that you will be able to just make the callback async and use await from it directly without any extra wrappers:

router.get('/:zipcode/:type/:rad', async (req) => {
  ...
  return await getUsers({ type, zips });
});

Note that as of December 2021, Express 5 is still in alpha release, not recommended for production use.

Answer 2

Instead of converting every callback function to Promise manually, it is easier to create a wrapper to support async/await.

Reference
https://strongloop.com/strongblog/async-error-handling-expressjs-es7-promises-generators/

function asyncWrapper(fn) {
  return (req, res, next) => {
    return Promise.resolve(fn(req))
      .then((result) => res.send(result))
      .catch((err) => next(err))
  }
}

Example Code

async createUser(req) {
  const user = await User.save(req.body)
  return user
}

router.post('/users', asyncWrapper(createUser))

Refactoring Your Code

function userGroup(type, zip) {
  return new Promise(resolve => {
    connection.query(`SELECT * FROM bt9.users WHERE zip = ${zip} AND type = ${type} AND display = 1`, function (err, result) {
      if(err) throw err;
      resolve(result);
    });
  });
}

async function getUsers({ type, zips }) {
  let userList = [];
  // [IMPORTANT]
  // - Replaced the for-loop to for-of-loop for await to work.
  // - This is not efficient because the `userGroup` function is run one by one serially.
  for (let zip of zips) {
    const users = await userGroup(type, zip);
    userList = userList.concat(users);
  }
  return userList;
}

router.get('/:zipcode/:type/:rad', asyncWrapper(async (req) => {
  const rad = req.params.rad;
  const zip = req.params.zipcode;
  let zips = zipcodes.radius(zip, rad);
  zips.push(zip);

  let type;
  if(req.params.type === 'bartenders') {
    type = 0;
  } else {
    type = 1;
  }

  return await getUsers({ type, zips });
}));

To further improve the efficiency, you should replace the for-of-loop inside getUsers with Promise.map offered by bluebird. Promise.map will run the promises in parallel.

async function getUsers({ type, zips }) {
  let userList = []
  const userListList = await Promise.map(zips, (zip) => {
    return userGroup(type, zip);
  });
  for (let users of userListList) {
    userList = userList.concat(users)
  }
  return userList;
}

Answer 3

To add on to Steven Spungin's answer here's the getUsers function refactored with Promise.all:

function getUsers({zips, type}) {
  return Promise.all(zips.map(zip => userGroup(type, zip)))
    .then(users => users.flat());
}

Alternatively, if you don't mind using a third-party module, you could drop in async-af and make use of its mapAF (alias map) method:

const aaf = require('async-af');
// ...
async function getUsers({zips, type}) {
  const userGroups = await aaf(zips).map(zip => userGroup(type, zip));
  return userGroups.flat(); // flat isn't yet part of 'async-af' but now that it's finalized for ES, I'm sure it'll be added soon.
}

Answer 4

You should not throw an error when you are not inside an async function.

function userGroup(type, zip) {
    return new Promise( (resolve,reject) => {
      connection.query(`SELECT * FROM bt9.users WHERE zip = ${zip} AND type = ${type} AND display = 1`, function (err, result) {
        if(err) return reject(err); //<- reject and return
        resolve(result);
      });
    });
  }

Also, you can use Promise.all with an array of promises instead of await inside each loop iteration. That would allow for parallel execution to your connection.