Promise is fulfilled on error

Question

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

I'm curious, what do you want the program to do if it fails to connect to the database? Exit? Send an error page to the user? Try to reconnect? Or you really want it to just log the error and hang around? — Gjorgi Kjosev, Mar 19 '14 at 20:50
possible duplicate of [Chained promises not passing on rejection](http://stackoverflow.com/questions/16371129/chained-promises-not-passing-on-rejection) — Bergi, Mar 20 '14 at 12:12

sabof · Answer 1 · 2014-03-19T19:20:05.873

then creates a new promise in some libraries. My guess would be that bluebird also creates a promise with catch. The way you have chained them, redisPromise refers to the "catch" promise, and not the first promise. And "catch" is indeed resolved.

If I'm right, this should work as expected:

var redisPromise = new Promise(function(resolve, reject) {
  redisClient.on('error', reject);
  redisClient.on('ready', resolve);
});

redisPromise.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");
});

Yeah, `catch` should be alias to `then(null, handler)`. But I think you have point here. I have to fiddle it little bit more, because using it like this, Bluebird propagates that error to top level and outputs to console. I don't like that behavior. — FredyC, Mar 19 '14 at 19:49

score 1 · Accepted Answer · answered Mar 19 '14 at 20:23

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 :/');
});

Sure, feel free to ask more questions it creates a good knowledge base :) — Benjamin Gruenbaum, Mar 19 '14 at 20:26

score 1 · Answer 3 · answered Mar 19 '14 at 20:52

Isn't what you want simply

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;
}).then(function() {
  log("This shouldn't be called when connection fails");
}).catch(function() {
  // THIS GETS CALLED - CORRECT
  log('failed connecting to database:', e.message);
});

Which is same as:

try {
   var redisClient = ...
   log.enabled && log('connected, version: %s', redisClient.server_info.redis_version);
   log("This shouldn't be called when connection fails");
} 
catch(e) {
   log('failed connecting to database:', e.message);
}

Yeah, it gives me some hard time to look at it like sychronous code, but hopefully I will get used to it. After [discussion](https://github.com/petkaantonov/bluebird/issues/156) at GH, it makes sense to me now. — FredyC, Mar 20 '14 at 22:05

Promise is fulfilled on error

3 Answers3