Never Mix Promises and Callbacks in NodeJS (spin.atomicobject.com)

Wouldn't using the second parameter of .then() to catch exceptions work instead of .catch()?

http://stackoverflow.com/a/33278420

  promise.then(function(){
    throw new Error('Oh no, not caught');
  }, function(err){

  });

  promise.then(function(){
    throw new Error('Oh no!');
  }).catch(function(err){
    // Caught it ;)
  });

  promise.then(function(){
    throw new Error('Oh no!');
  }).then(null, function(err){
    // Caught it ;)
  });

It's also an anti-pattern to not have an err as the first argument in an async callback.

Or ... return the promise in `fetchFirstUser`?

Mocha has support for returning promises in it() tests, so there's really no need to use callbacks.

Why was the solution to fallback to callbacks instead of just using promises all the way through? (Edit, removed mention of bluebird that would not have helped)

The problem here isn't that they are being swallowed, it is that the tests have a constraint that the callback be called only once, which is conveniently the behavior of a then applied to the promise you would return.

    // Promisifying a standard callback
    const myAsyncMethod = Bluebird.promisify(myCallbackMethod);

    // Or if you have weird callback passing and need
    // to manipulate the callback method being passed around
    function myAsyncMethod(...args) {
        return Bluebird.fromCallback(cb => myAsyncMethod(cb, ...args));
    }

    async function fetchFirstUser(iswhatIwant, callback) {
      const safeCallback = (user) => {
        try {
          callback(user);
        } catch (e) {
          console.error(e);
        }
      };

      try {
        const user = await knex.first().from("users");
        safeCallback(user);
        return user;
      } catch (e) {
        safeCallback(null);
        throw e;
      }
    }

    async doWithCallback(promise, callback) {
      const safeCallback = (error, result) => {
        if (callback == null) {
          return;
        }
        try {
          callback(null, result);
        } catch (e) {
          console.error(e);
        }
      };
      try {
        const result = await promise;
        safeCallback(null, result);
        return result;
      } catch (e) {
        safeCallback(e);
        throw e;
      }
    }

    async function fetchFirstUser(iswhatIwant, callback) {
      return doWithCallback(knex.first().from("users"), callback);
    }

or just use async which fixes both of these arguably broken solutions for callback hell.

Is there any typechecking library for JS that can warn of potentially "swallowed" promises? Does Flow do this?

    process.on("unhandledRejection", err => {
        console.error("Uncaught Promise Error: \n" + err.stack);
        console.error(err);
    });

Callbacks are simpler, easier to understand and explain. 'callback hell' is usually the result of 'poor code structure' that get's fixed with separated named functions.

Better never use Promises at all, it is no solution for a problem, instead it makes code only more ambiguous IMHO. And indeed mixing it with callbacks or async libraries makes it even worse..

I've once had a very nasty bug keeping me busy for days, where somewhere in a complex structure of multiple promises I somehow forgot to return a call to a resolve. With all these new Promise(), .then(), .catch(), resolve's and reject's nested it's very easy to miss one, and very hard to read where the bug sits.

Maybe it's just me, but I never had such a severe bug with the simplicity of the standard async callback pattern. I Never used promises again, still enjoying that decision.

The problem isn't mixins Promises and callbacks, in fact mixing the two is often completely necessary and people need to learn to do it correctly exactly to avoid problems like these.

The problem here is that chained Promise reactions, like .then(A).catch(B) can indeed call both A and B if A throws, which in this case is very bad if you can only call one of them.

The correct solution is to use an error handler in .then():

    function fetchFirstUser(iswhatIwant, callback) {
      knex.first().from("users").then((user) => {
        callback(user);
      }, (e) => {
        callback(null);
      });  
    }

But anytime you're invoking callbacks you also have to consider the case where the callback throws - which is one reason returning Promises is generally easier to get correct than invoking callbacks - so you _also_ need a .catch():

    function fetchFirstUser(iswhatIwant, callback) {
      knex.first().from("users").then((user) => {
        callback(user);
      }, (e) => {
        callback(null);
      }).catch((e) => {
        // oh no, callback() threw! bad callback!
        console.error(e);
      });
    }

This might seem like a lot, but I blame callbacks in general for the extra work that needs to be done. If you can avoid callbacks, do, because Promises all the way through would be so much simpler:

    function fetchFirstUser(iswhatIwant, callback) {
      return knex.first().from("users");
    }

And push error handling out to the initiator.

  function fetchFirstUser(isWhatIWant, callback) {
      knex.first().from('users')
          .then(user => ({ user }))
          .catch(e => ({ e }))
          .then(({ user, e }) => {
              if (user) {
                  callback(user);
              } else {
                  callback(e);
              }
          });
  }

    function fetchFirstUser(isWhatIWant, callback) {
      knex.first().from('users')
        .then((user) => ({ user }), (e) => ({ e }))
        .then(({ user, e }) => callback(e, user))
        .catch((e) => console.error(e));
    }

I just wrote a reply on Medium to this article: https://medium.com/@fpresencia/please-mix-promises-and-callb...

Takeaways:

- It has the wrong signature! The callback is the second parameter but is being passed as the first, so it will never be called.

- Even if that's a typo, don't catch what you intend to throw as an exception. Use the callback as the second parameter of the `then` and let it fail. The error will stil l be swallowed, but in a more predictable way.

- Better yet, use async/await if possible! It makes promises a joy to work with.

> Knex queries return Bluebird promises by default

I haven't kept up with JS trends recently. Are these promise wrapper libraries like Bluebird still relevant now that ES6 is out?

This is another great reason never to use promises.

"Callback Hell" is FUD spread by people who somehow missed the memo about the fact that you can add functions to your code. http://callbackhell.com explains it quite well.

In running away from "Callback Hell" the JavaScript community ran straight into "Control Structure Hell" which is far, far worse.

There is a point nobody here is making, which is that all the examples use the wrong signature for callbacks, which is function(err, result). The problem is not interop between the two, which is fairly easy - the problem arises when not using standard signatures. Seems like all the fuss could be avoided too because Mocha supports both promises and callbacks.

  function asCallback(promise, callback) { 
    return Promise.resolve(promise)
      .then(result => callback(null, result))
      .catch(callback) 
  }

  // returns a new fn that takes same args which returns promise instead
  function asPromise(fn) { 
    return (...args) => new Promise((resolve, reject) => {
      fn(...args, (err, result) => { 
        if (err) return reject(err); 
        resolve(result) 
      })
    })
  }

Weird things can happen with event emitters mixed up with promises too. I wrote a short piece with an example:

https://qubyte.codes/blog/promises-and-nodejs-event-emitters...

(The title is a bit overstated, since it's really about the specific example of unhandled error emissions in a promise chain.)

Regardless of the merits or problems of either post, a lesson is to be careful around promises when errors are involved.