João Pereira João Pereira - 18 days ago 6
Javascript Question

Express.js and Bluebird - Handling the promise chain

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

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);
  }
});