How to fix the promise anti pattern - Ghost Promise?

Question

I found the term "The Ghost Promise" here, which looks like my case.

I have the code like this:

return Q.Promise(function(resolve, reject) {
  firstFunctionThatReturnPromise()
  .then(function(firstResult) {
    _check(firstResult) ? resolve(firstResult) : return secondFunctionThatReturnPromise();
  })
  .then(function(secondResult) {
    console.log(secondResult);
    return thirdFunctionThatReturnPromise(secondResult);
  })
  .then(function(thirdResult) {
    resolve(thirdResult);
  })
  .catch(function(e) {
    reject(e)
  });
});

The problem is, even though the _check returns true, it still proceeds to the console.log command (which results in undefined).

In case the _check returns false, things work as expected.

So my question is:

• If the behavior described above is normal?
• If there is a more elegant way to handle this case?

Update 1: Many questioned that why I use Q.Promise instead of returning the result directly. It's because this is a generic function, shared by other functions.

// Usage in other functions
genericFunction()
.then(function(finalResult) {
   doSomething(finalResult);
})
.catch(function(err) {
   handleError(err);
});

Answer 1

First off, there's no reason to wrap a new promise around any of this. Your operations already return promises so it is an error prone anti-pattern to rewrap them in a new promise.

Second off, as others have said, a .then() handler has these choices:

1. It can return a result which will be passed to the next .then() handler. Not returning anything passes undefined to the next .then() handler.
2. It can return a promise whose resolved value will be passed to the next .then() handler or rejected value will be passed to the next reject handler.
3. It can throw which will reject the current promise.

There is no way from a .then() handler to tell a promise chain to conditionally skip some following .then() handlers other than rejecting.

So, if you want to branch your promise based on some condition logic, then you need to actually nest your .then() handlers according to your branching logic:

a().then(function(result1) {
    if (result1) {
        return result1;
    } else {
        // b() is only executed here in this conditional
        return b().then(...);
    }
}).then(function(result2) {
    // as long as no rejection, this is executed for both branches of the above conditional
    // result2 will either be result1 or the resolved value of b()
    // depending upon your conditional
})

So, when you want to branch, you make a new nested chain that lets you control what happens based on the conditional branching.

Using your psuedo-code, it would look something like this:

firstFunctionThatReturnPromise().then(function (firstResult) {
    if (_check(firstResult)) {
        return firstResult;
    } else {
        return secondFunctionThatReturnPromise().then(function (secondResult) {
            console.log(secondResult);
            return thirdFunctionThatReturnPromise(secondResult);
        })
    }
}).then(function (finalResult) {
    console.log(finalResult);
    return finalResult;
}).catch(function (err) {
    console.log(err);
    throw err;
});

Even if this is inside a genericFunction, you can still just return the promise you already have:

function genericFunction() {
    return firstFunctionThatReturnPromise().then(function (firstResult) {
        if (_check(firstResult)) {
            return firstResult;
        } else {
            return secondFunctionThatReturnPromise().then(function (secondResult) {
                console.log(secondResult);
                return thirdFunctionThatReturnPromise(secondResult);
            })
        }
    }).then(function (finalResult) {
        console.log(finalResult);
        return finalResult;
    }).catch(function (err) {
        console.log(err);
        throw err;
    });
}

// usage
genericFunction().then(...).catch(...)

Answer 2

The behavior is expected. When you chain your .then() statements, you cannot break out of the chain early except by throwing an error.

Your top-level promise (the one returned by Q.Promise()) gets resolved after _check(); but you actually have an inner promise chain that continues to execute.

By specification, then() returns a promise: https://promisesaplus.com/#point-40

You can see for yourself in the source code of Q: https://github.com/kriskowal/q/blob/v1/q.js#L899

For your desired behavior, you'll actually need another nested promise chain.

return Q.Promise(function(resolve, reject) {
  firstFunctionThatReturnPromise().then(function(firstResult) {
    if (_check(firstResult)) {
      resolve(firstResult);
    } else {
      return secondFunctionThatReturnPromise().then(function(secondResult) {
        console.log(secondResult);
        return thirdFunctionThatReturnPromise(secondResult);
      });
    }
  });
});

Answer 3

I have never used Q, but everything a promise returns is transformed into a promise and passed to the next .then(). Here, your first .then() don't return anything. So it returns undefined. So undefined is wrapped in a new Promise and passed to the next handler, where you get secondResult == undefined.

You can see it in action in the following CodePen : http://codepen.io/JesmoDrazik/pen/xOaXKE