36

I'm using the Q module for Node.js in attempts to avoid the "pyramid of doom" in scenarios where I have many steps. For example:

function doTask(task, callback)
{
    Q.ncall(task.step1, task)
    .then(function(result1){
        return Q.ncall(task.step2, task);
    })
    .then(function(result2){
        return Q.ncall(task.step3, task);
    })
    .fail(callback).end();
}

Essentially this seems to work; if an error is thrown by any of the task steps, it is passed to the callback (though I would be welcome to improvements, as I am new to node.js promises). However, I have a problem when I need to abort the task-chain early. For example, if result1 is successfully returned I might want to call the callback early and abort the rest, but my attempts to do so are failing...

function doTask(task, callback)
{
    Q.ncall(task.step1, task)
    .then(function(result1){
        if(result1)
        {// the rest of the task chain is unnecessary 
            console.log('aborting!');
            callback(null, result1);
            return null;
        }
        return Q.ncall(task.step2, task);
    })
    .then(function(result2){
        console.log('doing step 3...');
        return Q.ncall(task.step3, task);
    })
    .fail(callback).end();
}

In this example, I see both "aborting!" and "doing step 3..." printed.

I'm sure I'm merely misunderstanding some basic principles here, so would appreciate any help. Thanks!

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
Zane Claes
  • 14,732
  • 15
  • 74
  • 131
  • One solution I found is to create a separate promise chain after the first chain might break. Than is, in the above example, the .then statement with result2 becomes attached to the Q.ncall for step2, instead of being attached to the original promise. HOWEVER, the major downside here is that it gets rid of one of the major benefits for Q in my opinion: avoiding the pyramid of doom! It is still better than no promises at all, but I don't like the solution... – Zane Claes Jul 03 '12 at 13:30

3 Answers3

39

This is a case where you will need to branch, which does mean either nesting or creating a subroutine.

function doTask(task, callback) {
    return Q.ncall(task.step1, task)
    .then(function(result1) {
        if (result1) return result1;
        return Q.ncall(task.step2, task)
        .then(function(result2) {
            return Q.ncall(task.step3, task);
        })
    })
    .nodeify(callback)
}

Or

function doTask(task, callback) {
    return Q.ncall(task.step1, task)
    .then(function(result1) {
        if (result1) {
            return result1;
        } else {
            return continueTasks(task);
        }
    })
    .nodeify(callback)
}

function continueTasks(task) {
    return Q.ncall(task.step2, task)
    .then(function(result2) {
        return Q.ncall(task.step3, task);
    })
}
Onur Yıldırım
  • 32,327
  • 12
  • 84
  • 98
Kris Kowal
  • 3,866
  • 2
  • 24
  • 24
  • 1
    Is this the best approach for branching? It seems this introduces indentation again when there are multiple branches. Here's an [example](https://gist.github.com/svenjacobs/3f42bbaf4cbabe2b58b5) where I perform multiple file operations using [q-io](https://github.com/kriskowal/q-io). I first check if a dir exists, list the files looking for a certain file, and delete it if just one matching file is found. There are multiple if-conditions in there that should abort the chain. I use a special return value to check for that case but have to check it in every function. Is this a good approach? – Sven Jacobs May 16 '14 at 15:38
  • 4
    @SvenJacobs what you’re describing in that example is a good case for exceptions. Consider https://gist.github.com/kriskowal/e98774443eb0f1653871 – Kris Kowal May 24 '14 at 15:12
  • 3
    I still have an issue with this approach because it makes it harder for error handling. Throwing within a promise chain (Calvin Alvin's answer) allows to have a single `.fail()` that takes care of any error during the flow. Writing promises this way (branching) takes me back to the callback hell. – Pedro Aug 19 '15 at 04:02
  • 1
    @KrisKowal Thank you for creating the Q library, I've been using the bluebird library today with waterline, and unfortunately after spending hours on trying to terminate the call early I arrived at the same conclusion that Calvin did years ago - just use throw to jump directly to the catch. Can you explain why doing that is bad? Not a purist here, but this is what I ended up with - https://gist.github.com/yisyang/047fd95e8c9ffc1af561 – Scott Yang Sep 07 '15 at 11:12
20

Any errors that are thrown within the promise chain will cause the entire stack to be aborted early and control is given to the error-back path. (in this case, the fail() handler) When you detect a certain state which causes you to want to abort the promise chain, then just throw a very specific error, which you trap in the error-back and ignore (if you so choose)

function doTask(task, callback)
{
    Q.ncall(task.step1, task)
    .then(function(result1){
        if(result1 == 'some failure state I want to cause abortion')
        {// the rest of the task chain is unnecessary 
            console.log('aborting!');
            throw new Error('abort promise chain');
            return null;
        }
        return Q.ncall(task.step2, task);
    })
    .then(function(result2){
        console.log('doing step 3...');
        return Q.ncall(task.step3, task);
    })
    .fail(function(err) {
        if (err.message === 'abort promise chain') {
            // just swallow error because chain was intentionally aborted
        }
        else {
            // else let the error bubble up because it's coming from somewhere else
            throw err;
        } 
    })
    .end();
}
Calvin Alvin
  • 2,448
  • 2
  • 16
  • 15
  • 19
    You're using exceptions for control flow, and this is not usually recommended. The solution given by Kris Kowal avoids that problem. – Gjorgi Kjosev Oct 09 '13 at 00:48
  • 4
    `return null` is not necessary after the `throw` – Pepijn Nov 26 '15 at 12:03
  • I don't like exceptions as a `break;` equiv either, but in this case it's the only option without creating incomprehensible and ugly random functions and return blocks. – David Dombrowsky Jan 23 '20 at 17:19
2

I believe you only have to reject the promise to break out of the promise chain.

https://github.com/kriskowal/q/wiki/API-Reference#qrejectreason

also it seems like .end() has been changed to .done()

function doTask(task, callback)
{
    Q.ncall(task.step1, task)
    .then(function(result1){
        if(result1)
        {// the rest of the task chain is unnecessary 
            console.log('aborting!');
            // by calling Q.reject, your second .then is skipped,
            // only the .fail is executed.
            // result1 will be passed to your callback in the .fail call
            return Q.reject(result1);
        }
        return Q.ncall(task.step2, task);
    })
    .then(function(result2){
        console.log('doing step 3...');
        return Q.ncall(task.step3, task);
    })
    .fail(callback).done();
}
George
  • 1,552
  • 1
  • 17
  • 26