5

I have a function which is going to look up the cache if it finds anything, otherwise then then it will go ahead and fetch the data and set the cache. It's pretty standard. I'm wondering if the error occurs at the most inner function, will it keep bubble up to the outer most Promise? So, I can just have one catch instead of one.

Here's my code.

I'm using Bluebird

 var _self = this;
    return new Promise(function(resolve, reject) {
      _self.get(url, redisClient).then(function getCacheFunc(cacheResponse) {
        if(cacheResponse) {
          return resolve(JSON.parse(cacheResponse));
        }
        webCrawl(url).then(function webCrawl(crawlResults) {
          _self.set(url, JSON.stringify(crawlResults), redisClient);
          return resolve(crawlResults);
        }).catch(function catchFunc(error) {
          return reject(error); // can I delete this catch
        });
      }).catch(function getCacheErrorFunc(cacheError) {
        return reject(cacheError); // and let this catch handle everything?
      });
    });
toy
  • 11,711
  • 24
  • 93
  • 176
  • 1
    It won't, but you can shorten it quite a bit by doing just `.catch(reject);` – adeneo Feb 11 '16 at 17:17
  • 1
    No, you cannot, as you didn't `return` the promise from your `then` callback. And you should [avoid the `Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Feb 11 '16 at 17:42

2 Answers2

5

Yes, it is possible to have a single .catch(...) for deeply-nested Promises. The trick: you can resolve a Promise with another Promise. This means that you can refactor your code to:

var _self = this;
_self.get(url, redisClient)
  .then(function(cacheResponse) {
    if(cacheResponse) {
      // Resolve the Promise with a value
      return JSON.parse(cacheResponse);
    }

    // Resolve the Promise with a Promise
    return webCrawl(url)
      .then(function(crawlResults) {
        _self.set(url, JSON.stringify(crawlResults), redisClient);

        // Resolve the Promise with a value
        return crawlResults;
      });
  })
  .catch(function(err) {
    console.log("Caught error: " + err);
  });

Note: I also removed your outermost Promise declaration. This was no longer necessary since _self.get(...) already returned a Promise.

Brian Lauber
  • 121
  • 3
  • In the catch method should I throw the `err` so the method that's calling `_self.get` can use `catch` to get the error? – toy Feb 11 '16 at 18:49
  • @toy you only use `.catch` if you want to shallow the error or if you want to react on it, if you only place a `throw` inside of it and nothing else, then you should omit the the whole `catch` block. – t.niese Feb 11 '16 at 18:51
  • 1
    Agreed with @t.niese: In practice, you should just omit the `.catch(...)` block unless your code in explicitly handling the exception. This way, it will be the caller's responsibility to handle the error. (In the answer above, I was just demonstrating that the `catch(...)` would ultimately be applied to the outer Promise rather than the inner Promise) – Brian Lauber Feb 12 '16 at 00:57
3

Assuming that .get returns a Promise you would write it that way:

 var _self = this;
 return _self.get(url, redisClient).then(function(cacheResponse) {
   if (cacheResponse) {
     return JSON.parse(cacheResponse);
   } else {
     return webCrawl(url).then(function(crawlResults) {
       _self.set(url, JSON.stringify(crawlResults), redisClient);
       return crawlResults;
     })
   }
 });

There is no need to introduce a new Promise because you already get one from your _self.get

t.niese
  • 39,256
  • 9
  • 74
  • 101