[Fixed] How to write and handle NodeJS promises elegantly

Issue

I am using ExpressJS for APIs and use a lot of promises for asynchronous requests like external API calls, database queries. In most cases its fine but when i have a lot of calls, the code becomes messy because of a lot of .then -> .catch statements. Here’s an example of a login API which does API calls, Database queries

const loginUser = (req, res) => {
  const { body } = req;

  console.log('Login');

  if (!body) {
    return res
      .status(400)
      .json({ success: false, error: 'request body is empty' });
  }

  if (!body.authCode) {
    return res
      .status(400)
      .json({ success: false, error: 'authCode missing in the body' });
  }

  getLinkedInProfile(body.authCode)
    .then((linkedInProfile) => {
      User.findOne({ linkedInId: linkedInProfile.linkedInId })
        .then((user) => {
          if (user) {
            Token.findOne({ userId: user._id })
              .then((token) => {
                // if token isn't in our DB, store
                if (!token) {
                  // encrypt information
                  const t = util.refreshToken(user._id);

                  Token.create({ refreshToken: t, userId: user._id });
                }
                // send the access token
                const accessToken = util.accessToken(user._id);

                return (
                  res
                    // .cookie('accessToken', accessToken, {
                    //   sameSite: 'none',
                    //   secure: true,
                    // })
                    .json({
                      success: true,
                      message: 'Login Successful',
                      token: accessToken,
                      user: {
                        _id: user._id,
                        userType: user.userType,
                      },
                    })
                );
              })
              .catch((err) => {
                console.log(err);
                return res.status(500).json({
                  success: false,
                  error: 'Error in find token',
                });
              });
          } else {
            return res.status(200).json({
              success: false,
              message: 'User does not exist',
            });
          }
        })
        .catch((err) => {
          console.log(err);
          return res.status(500).json({
            success: false,
            error: 'Unable to query database',
          });
        });
    })
    .catch((err) => {
      console.log(err);
      return res.status(500).json({
        success: false,
        error: 'Unable to authenticate linkedIn profile',
      });
    });
};

Can someone help review this / suggest ways to make the code more clean

Solution

Promises do require a bit of extra work if you want a custom error at every step of the way. I typically try to do several things to simplify and remove repetition:

1. Send all errors from one place, rather than all through the code.
2. Let errors propagate up to the top level try/catch or catch.
3. Create custom properties on the Error object to keep track of what you want the final error result to be so you can throw that Error and then intercept it later and know what you want the error response to be in one central place.
4. Create a helper function to construct this custom error.
5. Log any actual exceptions or rejections so we can always see what the original error was, especially if we’re changing it into our own custom message. This is super important when debugging on the server where you want to see what the actual low level error is.

Then, because you want a custom error at every step of the way, it just takes a lot of custom error catching. Even though I’m using await in some places, I use .catch() for the custom error production because it vastly simplifies the code. Some people don’t like seeing await and .catch() used together, but if you write this code without .catch() you’ll immediately see why this is so much simpler so this is one exception I allow myself.

And, lastly using async and await flattens the code and makes the flow a lot simpler. It also makes it easy to get all the errors to flow to the one top level catch if we just throw them.

Here’s the code:

// utility function to construct an error object, 
// set some properties and then throw the error object
function throwError(status, msg, e) {
    if (e) {
        // log the actual error before we change it to something else
        console.log(e);
    }
    const err = new Error(msg);
    err.json = { success: false, error: msg };
    err.status = status;
    throw err;
}

const loginUser = async (req, res) => {
    try {
        const { body } = req;

        console.log('Login');

        if (!body) {
            throwError(400, 'request body is empty');
        }

        if (!body.authCode) {
            throwError(400, 'authCode missing in the body');
        }

        const linkedInProfile = await getLinkedInProfile(body.authCode)
            .catch(err => throwError(500, 'Unable to query database', err));

        const user = await User.findOne({ linkedInId: linkedInProfile.linkedInId })
            .catch(err => throwError(500, 'Unable to authenticate linkedIn profile', err));

        if (!user) {
            throwError(200, 'User does not exist');
        }
        const token = await Token.findOne({ userId: user._id })
            .catch(err => throwError(500, 'Error in find token', err));

        // if token isn't in our DB, store
        if (!token) {
            // encrypt information
            const t = util.refreshToken(user._id);
            Token.create({ refreshToken: t, userId: user._id });
        }
        // send the access token
        const accessToken = util.accessToken(user._id);
        res.json({
            success: true,
            message: 'Login Successful',
            token: accessToken,
            user: {
                _id: user._id,
                userType: user.userType,
            },
        });

    } catch (e) {
        // send error response here from all prior errors
        let status = e.status | 500;
        let json = e.json | { success: false, error: e.message };
        res.status(status).json(json);
    }
}