CODEWITHSUNDEEP
javascriptexpressmongooseecmascript-6promise
Elias Garcia
Elias Garcia

Reputation: 7282

How can I fix this ES6 promise chain in Express?

I'm starting with Promise chaining and I'm trying some code using express and mongoose in node. This is my code:

const register = (req, res, next) => {
  User.findOne(req.params.email).exec().then(user => {
    if (user) {
      return res.status(409).json({ error: 409 });
    }
    return User.create(req.body);
  }).then(user => {
    const token = jwt.sign({ sub: user._id }, 'test-secret');
    return res.status(200).json({ token });
  }).catch(err => {
    return next(err);
  });
};

This is a simplified code to register a user and send him a token. What I want to do is, first, check if the user is already registered and, if not, register it.

As you can see, the line 6 is wrong I think, because I'm not returning any Promise, so after the line 4, the code continuos executing. I want to avoid the callback hell, how can I accomplish this? Thanks

Upvotes: 1

Views: 65

Answers (2)

salezica
salezica

Reputation: 76929

The fact that you're not returning a Promise is not important. Return values from then() and catch() are automatically wrapped in Promise objects.

The problem is that your flow is linear (it follows a straight path), but you are trying to branch so that some parts only run in some cases. You need two execution paths:

const register = (req, res, next) => {
  User.findOne(req.params.email).exec().then(user => {
    if (user) {
      // Path 1 stops here:
      return res.status(409).json({ error: 409 });

    } else {
      // Path 2 continues down this 2nd Promise chain:
      return User.create(req.body).then(user => {
        const token = jwt.sign({ sub: user._id }, 'test-secret');
        return res.status(200).json({ token });
      })
    }

  }).catch(err => {
    // Both paths converge on this error handler
    return next(err);
  });
};

If you want to avoid deep nesting, you have two options:

  1. Encapsulate the different paths into functions, so you can then do something like this:

    if (user) {
  return sendHttpError(res, 409)
} else {
  return sendNewUser(res, req.body)
}

Perhaps only the sendHttpError function is necessary, and the other possible path can be the main body of your route. This is pretty standard.

  1. Since one of your branches is only dealing with error cases, you can throw an Error and catch it below, or in an error-handling middleware. It would look like this:

    if (user) {
  // This throw will abort execution of everything that follows. By
  // using a custom Error class, you can then handle it appropriately
  // in a catch() handler or Express middleware:
  throw new APIError(409)
}

// ... create user and proceed normally

This is also a very common pattern.

Upvotes: 3

Erazihel
Erazihel

Reputation: 7605

Just throw an error if the user already exists to go directly to the catch function.

const register = (req, res, next) => {
  User.findOne(req.params.email).exec().then(user => {
    if (user) {
      throw new Error({ error: 409, msg: 'User already exists' });
    }
    return User.create(req.body);
  }).then(user => {
    const token = jwt.sign({ sub: user._id }, 'test-secret');
    return res.status(200).json({ token });
  }).catch(err => {
    return next(err);
  });
};

Upvotes: 2

Related Questions