Express.js and Bluebird - Handling the promise chain

Question

In a backend API I have a login route which should perform the following sequence of actions:

• Given an username and password, try to authenticate the user against an Active Directory. If authentication has failed reply with status 401. If success, continue.

• Look for an user with the given username in the database. If not found reply with status 403, otherwise continue.

• Find if the user document has some details like email, display name, etc (in case this is not the first time logging in). If yes reply with the user object, otherwise continue.

• Get user details from the Active Directory and update the user object in the database. Reply with the updated object.

Code:

router.post('/login', (req, res, next) => {

  // capture credentials
  const username = req.body.username;
  const password = req.body.password;
  let user = null;

  // authenticate
  ad.authenticate(username, password)
    .then((success) => {
      if (!success) {
        res.status(401).send(); // authentication failed
        next();
      }
      return User.findOne({ username }).exec();
    })

    .then((found) => {
      if (!found) {
        res.status(403).send(); // unauthorized, no account in DB
        next();
      }
      user = found;
      if (user.displayName) {
        res.status(201).json(user); // all good, return user details
        next();
      }
      // fetch user details from the AD
      return ad.getUserDetails(username, password);
    })

    .then((details) => {
      // update user object with the response details and save
      // ...
      return user.save();
    })

    .then((update) => {
      res.status(201).json(update); // all good, return user object
      next();
    })

    .catch(err => next(err));

});

Now I had this running with callbacks but it was really nested. So I wanted to give Bluebird promises a try, but I have two problems:

• Looks chaotic, any better way to chain the calls and handle responses?

• Whenever I call next() to stop the request after replying, the execution continues to the other .then(). Although the client receives the correct response, in the server log I find that the execution have continued. For example, if there is no account in DB for a given user, the client receives the 403 response but in the server log I see an exception failed to read property displayName of null, because there was no user and it should have stopped in the next() after res.status(403).send();.

Answer 1

Best use if/else to make clear what branches will execute and which won't:

ad.authenticate(username, password).then((success) => {
  if (!success) {
    res.status(401).send(); // authentication failed
  } else {
    return User.findOne({ username }).exec().then(user => {
      if (!user) {
        res.status(403).send(); // unauthorized, no account in DB
      } else if (user.displayName) {
        res.status(201).json(user); // all good, return user details
      } else {
        // fetch user details from the AD
        return ad.getUserDetails(username, password).then(details => {
          // update user object with the response details and save
          // ...
          return user.save();
        }).then(update => {
          res.status(201).json(update); // all good, return user object
        });
      }
    });
  }
}).then(() => next(), err => next(err));

The nesting of then calls is quite necessary for conditional evaluation, you cannot chain them linearly and "break out" in the middle (other than by throwing exceptions, which is really ugly).

If you don't like all those then callbacks, you can use async/await syntax (possibly with a transpiler - or use Bluebird's Promise.coroutine to emulate it with generator syntax). Your whole code then becomes

router.post('/login', async (req, res, next) => {
  try {
    // authenticate
    const success = await ad.authenticate(req.body.username, req.body.password);
    if (!success) {
      res.status(401).send(); // authentication failed
    } else {
      const user = await User.findOne({ username }).exec();
      if (!user) {
        res.status(403).send(); // unauthorized, no account in DB
      } else if (user.displayName) {
        res.status(201).json(user); // all good, return user details
      } else {
        // fetch user details from the AD
        const details = await ad.getUserDetails(username, password);
        // update user object with the response details and save
        // ...
        const update = await user.save();
        res.status(201).json(update); // all good, return user object
      }
    }
    next(); // let's hope this doesn't throw
  } catch(err) {
    next(err);
  }
});