Search code examples
javascripterror-handlingpromisebluebird

Promise is fulfilled on error


Well, I thought I understand Promises already, but seems I am missing something on this...

var redisPromise = new Promise(function(resolve, reject) {
  redisClient.on('error', reject);
  redisClient.on('ready', resolve);
}).then(function() {
  // THIS ISN'T CALLED - CORRECT
  log.enabled && log('connected, version: %s', redisClient.server_info.redis_version);
  return redisClient;
}).catch(function() {
  // THIS GETS CALLED - CORRECT
  log('failed connecting to database:', e.message);
});

redisPromise.then(function() {
  log("This shouldn't be called when connection fails");
});

For the case when connection to Redis fails I would expect the returned Promise to be rejected. However for some reason it's fulfilled. Am I missing something here?

I am using Bluebird implementation. Could it be possibly some bug in there ? I kinda doubt that, it all seems very well documented and making sense ... on paper.

RESOLVED

Full discussion at https://github.com/petkaantonov/bluebird/issues/156


Solution

  • Copying answer from GH issue including Domenic's code sample:

    Domenic gave the following example: Think of the equivalent synchronous code:

    try {
       ...
    } catch (e) {
      // THIS GETS CALLED - CORRECT
      log('failed connecting to database:', e.message);
    }
    
    log('This shouldn't be called when connection fails :/');
    

    and you can see why your expectations don't work so well.

    To add to Domenic's excellent analog. If you want to signal a fail in execution and handle it, you need to rethrow:

    try {
       ...
    } catch (e) {
      // THIS GETS CALLED - CORRECT
      log('failed connecting to database:', e.message);
      throw e;
    }
    

    In your case:

    }).catch(function(e) {
      log('failed connecting to database:', e.message);
      throw e;
    });
    

    If you want to signal the type of exception - throw your own type of failure. The analogy for the possibly unhandled rejection is what happens when you throw in synchronous code and don't handle it. It defaults to log it to the console.

    Here is an alternative solution I don't really like:

    var redisPromise = new Promise(function(resolve, reject) {
      redisClient.on('error', reject);
      redisClient.on('ready', resolve);
    }).then(function() {
      log.enabled && log('connected, version: %s', redisClient.server_info.redis_version);
      return redisClient;
    });
    redisPromise.catch(function() { 
      log('failed connecting to database:', e.message);
    });
    
    redisPromise.then(function() { 
      log('This shouldn't be called when connection fails :/');
    });