Handling promise errors in chain, am I too eager?

Question

Fairly new to angular and promises in general and I felt like I was doing everything right, until I read about common promise anti-patterns. I also noticed my code was getting really difficult to follow and read.

BTW, using Angular's Q implementation.

In this chain, am I handling error correctly? Should I use catch instead? How would I do that? Can I make my code more succinct?

EDIT: Couldn't find a question that talked about how to handle conditional promises. All the other stackoverflow questions were very simple without any complex chaining like my example. Hope this question helps someone

function importantPromise(){
    var deferred = $q.defer();
    var importantVariable;

    topLevelPromise().then(function(result) {
        var secondResult = doSomeWork(result, importantVariable);
        secondPromise(secondResult)
            .then(function(result) {
                deferred.resolve(result);
            }, function(error) {
                deferred.reject(error);
            });
    }, function(error) {
        conditionalPromise(importantVariable)
            .then(function(result) {
                deferred.resolve(result);
            }, function(error) {
                deferred.reject(error);
            });
    });
    return deferred.promise;
}

//Actually using the promise
importantPromise().then(function(result){
    handleResult(result);
}, function(error){
    handleError(error);
});

You shouldn't be using deferreds at all. Instead, return promises to build a chain. — SLaks, Jun 18 '15 at 14:34
But the promises depends on the previous promise? If I return the final promise won't I be losing the chance to handle the error of the `topLevelPromise()` ? — ObjectiveTruth, Jun 18 '15 at 14:37
`return secondFunction().then(function() { return result; })` — SLaks, Jun 18 '15 at 14:43
Interesting, maybe what I'm confused about is what happens to the error of what you wrote? Does it get bubbled up if I don't specify the second argument of `then()` ? — ObjectiveTruth, Jun 18 '15 at 14:54
Yes; if you don't specify an error handler, errors are returned as-is. — SLaks, Jun 18 '15 at 14:56

Michael Lorton · Accepted Answer · 2015-06-19T00:51:38.953

1

Funny, I just sent out an email to my team about exactly this mistake. You are making things far too hard for yourself. Just do this:

function importantPromise(){
    var importantVariable;

    return topLevelPromise().then(function(result) {
      var secondResult = doSomeWork(result, importantVariable);
      return secondPromise(secondResult);
    }).catch(function(error) {
      return conditionalPromise(importantVariable);
    });
}

This is the essence of promise-chaining.

You are probably thinking, "No, if I do this, my handleResult and handleError functions will get called prematurely, with promises instead of final values!"

No, no, no, porpoise-puss. Promise-chaining means that if a resolution function returns a promise, that promise is itself resolved and its value is passed to the next resolution-function in the chain. Try it.

edited Jun 19 '15 at 00:51

answered Jun 18 '15 at 15:03

Michael Lorton

43,060
26
103
144

Wow, that was defiantly the missing piece, thank you sir! – ObjectiveTruth Jun 18 '15 at 15:50
Then accept the answer! – Michael Lorton Jun 18 '15 at 16:36
oops, first time doing this, thanks – ObjectiveTruth Jun 18 '15 at 17:24
@Malvolio: But [that's not the same](http://stackoverflow.com/a/24663315/1048572)! I think OP did use the pattern on purpose. – Bergi Jun 19 '15 at 01:03
@Bergi, it cannot be the same: it's better! The previous version would only catch errors in `topLevelPromise()`; this version also catches any problems with `secondPromise()`. Bizarrely, as I noted I was talking to my team about the original issue yesterday, then today, 10 minutes after I read that blog post, I had exactly that problem in my code (an exception in the success handler not being caught). – Michael Lorton Jun 19 '15 at 04:42
It seems to me that the intention was to execute `conditionalPromise(…)` only if `topLevelPromise()` failed - not always we want to just catch all exceptions, but actually handle specific errors. – Bergi Jun 19 '15 at 05:03
In that case, I think (but am not sure) that you use the `.then(fulfill, reject)` form. All the other structures I can think of, either `conditionalPromise()` would receive errors from `secondPromise()` or `secondPromise()` would receive output from `conditionalPromise()`. – Michael Lorton Jun 19 '15 at 07:46
@Bergi @Malvolio I had done the original code to catch all the errors. At least that was my intention. Having looked up more on this subject. I can't really see a situation where you would use `function(error)` over `.catch(error)`. Unless its some edge case where you want to NOT catch the `throwables` in the success case. Am i thinking of it right? – ObjectiveTruth Jun 19 '15 at 14:11
1

This ticket is surreally tracking my life: this morning I had a problem where a 404-error (file not found) in one step in the middle of a 10-step-long chain of promises could be handled with a special intervention, but all other errors had to exit out to an logged warning. – Michael Lorton Jun 19 '15 at 14:49
1

@Malvolio, I've been playing around with both styles all morning and there is definitely a BIG distinction and not knowing the difference can be really bad. I just knocked out some bugs I couldn't explain in my code by revisiting it – ObjectiveTruth Jun 19 '15 at 15:08
`.catch(f)` is sugar for `.then(null, f)`. The difference between `.then(g,f)` and `.then(g).catch(f)` is that the latter will _also_ get any errors from `g`. – Michael Lorton Jun 19 '15 at 19:12

Handling promise errors in chain, am I too eager?

1 Answers1