Avoid Multiple Returns Looped In Javascript - Async / Await To Solve Callback Pyramid Or Callback Hell,

I have this code, with a lot of blocks of returns, by example SignUp() connectors.js const connectors = { Auth: { signUp(args) { return new Promise((resol

Solution 1:

This code needs to be refactored in a number of ways. First, you really, really don't want to mix promises and plain async callbacks in the same logic flow. It makes a mess and wrecks a lot of the advantages of promises. Then, you have an anti-pattern going using promises inside of new Promise(). Then, you have more nesting that is required (you can chain instead).

Here's what I'd suggest:

functionencryptPasswordPromise(pwd) {
    returnnewPromise((resolve, reject) => {
        encryptPassword(pwd, (err, hash) => {
            err ? reject(newError("The password could not be hashed.")) : resolve(hash);
        });
    });
}

const connectors = {
    Auth: {
        signUp(args) {
            // Validate the datalet err;
            if (!args.email) {
                err = {code: 'email.empty', message: 'Email is empty.'};
            } elseif (!isEmail(args.email)) {
                err = {code: 'email.invalid', message: 'You have to provide a valid email.'};
            } elseif (!args.password) {
                err = {code: 'password.empty', message: 'You have to provide a password.'};
            }
            if (err) {
                returnPromise.reject(err);
            } else {
                returnencryptPasswordPromise(args.password).then(hash => {
                    args.password = hash;
                    returnUser.create(args);
                }).then((user) => {
                    returncreateToken({id: user._id, email: user.email});
                }).catch(err2 => {
                    if (err2.code === 11000) {
                        thrownewError({code: 'user.exists', message: 'There is already a user with this email.'});
                    } else {
                        throw err2;
                    }
                });
            }
        }
    }
};

Summary of Changes:

1. Collect all errors initially and do return Promise.reject(err) in one place for all those initial errors.
2. Promisify encryptPassword() so you aren't mixing regular callbacks with promise logic flow and you can then properly return and propagate errors.
3. Removing wrapping of the whole code with return new Promise() since all your async operations are now promisified so you can just return promises directly. This really helps with proper error handling too.
4. Undo unneccessary nesting and just chain instead.
5. Remove Object.assign() as there did not appear to be a reason for it.

---

In your edit, you asked for an explanation of this code segment:

returnencryptPassword(args.password, (err, hash) => {
  if (err) {
    returnreject(newError('The password could not be hashed.'));
  }

  returnUser.create(Object.assign(args, { password: hash }))
    .then((user) => {
    .catch((err2) => {

      returnreject(err2);

1. This code calls encryptPassword().
2. It returns the result of that which is probably undefined so all the return is doing is controlling program flow (exiting the containing function), but since there is no code after that return at the same level, that's unnecessary.
3. You pass a callback to encryptPassword(). That callback is asynchronous, meaning it is called some time later.
4. The callback gets two arguments: err and hash. In the node.js async calling convention, if the first argument is truthy (e.g. contains some truthy value), then it represents an error. If it is falsey (usually null), then there is no error and the second argument contains the result of the asynchronous operation.
5. If there was an error, the parent promise is rejected and the callback is exited. Again, there is no return value from reject so the return is just for exiting the callback at that point (so no other code in the callback runs).
6. Then, another asynchronous operation User.create() is called and it is passed the args object with a new password set in it.
7. User.create() returns a promise so its result is captured with a .then() method and a callback passed to that.
8. Some time later, when the asynchronous User.create() finishes, it will resolve its promise and that will cause the .then() callback to get called and the result will be passed to it. If there's an error in that operation, then the .then() callback will not be called, instead the .catch() callback will be called. In that .catch() callback, the parent promise is rejected. This is a promise anti-pattern (resolving a parent promise inside another promise) because it is very easy to make mistakes in proper error handling. My answer shows how to avoid that.

Solution 2:

Extending @jfriend00's answer, here's an approach that uses the ES2017 async / await syntax to flatten the callback pyramid or callback hell, however you prefer to call it:

const encryptPasswordPromise = require('util').promisify(encryptPassword)

const connectors = {
  Auth: {
    async signUp (args) {
      const { email, password } = args
      // Validate the datalet err

      if (!email) {
        err = { code: 'email.empty', message: 'Email is empty.' }
      } elseif (!isEmail(email)) {
        err = { code: 'email.invalid', message: 'You have to provide a valid email.' }
      } elseif (!password) {
        err = { code: 'password.empty', message: 'You have to provide a password.' }
      }

      if (err) {
        throw err
      }

      let hash

      try {
        hash = awaitencryptPasswordPromise(password)
      } catch (err) {
        thrownewError('The password could not be hashed.')
      }

      const { _id: id, email } = awaitUser.create(Object.assign(args, { password: hash }))

      try {
        returncreateToken({ id, email })
      } catch (err) {
        if (err.code === 11000) {
          throw { code: 'user.exists', message: 'There is already a user with this email.' }
        } else {
          throw err
        }
      }
    }
  }
}

module.exports = connectors

Rather than handwrite my own promisified promise-based encryptPassword(), I opted to use a node.js builtin transformation function for that called util.promisify().

Overall, there are still a few improvements that could be made like moving the validation of the signUp() arguments to a separate function, but none of that is related to flattening the "callback hell" anti-pattern that gave rise to promise-based control-flow and async/await syntax.

Solution 3:

First things first. There are no loops in your code.

If you're coming from PHP, then I would guess Javascript's asynchronous execution model looks alien to you. I would suggest learning more about Javascript.

Learn the following in the following order:

1. Callbacks
2. Promises
3. Generators
4. Async/Await

Those are the different approaches on how to handle Javascript's asynchronous execution model starting from the most basic (callbacks) to the most modern approach, "async/await".

Async/await would be the most readable but it would be better if you understand how async/await came to be since they are easy to use the wrong way if you don't understand what you're doing.