Javascript Promise Pattern - differentiate error

问题:

Consider this block of code:

    getUser(userId)
    .catch(function(error){
        crashreporter.reportError('User DB failed', error);
        // show user a generic error
    })
    .then(function(user) {
        return chargeCreditCard(user);
    })
    .catch(function(error){
        crashreporter.reportError('Credit card failed', error);
        // show user an error saying that their credit card got rejected
    })

Obviously, the problem with this is that the THEN(USER) block gets executed if User DB fails. Another option is to move the first catch block to the end of the chain. However, that causes another issue! We won't be able to differentiate whether the error comes from the User DB or CreditCard.

Is the following pattern, which I think solves the problem, considered a Promise Anti Pattern? Is there a better way of doing this? The problem that I see with this, is that you can end up in a semi-callback hell.

    getUser(userId)
    .then(function(user) {
        return chargeCreditCard(user)
              .catch(function(error){
                  crashreporter.reportError('Credit card failed', error);
                  // show user an error saying that their credit card got rejected
              });
    })
    .catch(function(error){
        crashreporter.reportError('User DB failed', error);
        // show user a generic error
    })

---

Edit: I guess I haven't been very clear. What if there are more THEN blocks, like what below. The problem is that once you hit one ERROR, you don't want the chain to continue at all.

getUser(userId)
    .then(function(user) {
        return chargeCreditCard(user);
    }, function(error){
        crashreporter.reportError('User DB failed', error);
        // show user a error 1
    })
    .then(function(chargeId) {
        return saveChargeId(chargeId);
    }, function(error){
        crashreporter.reportError('ChargeId DB failed', error);
        // show user a error 2
    })
    .then(function(chargeHistoryId) {
        return associateChargeToUsers(chargeHistoryId);
    }, function(error){
        crashreporter.reportError('chargeHistoryId DB failed', error);
        // show user a error 3
    })
    .catch(function(error){
        crashreporter.reportError('Credit card failed', error);
        // show user a error 4
    })

回答1:

Is the following pattern, which I think solves the problem, considered a Promise Anti Pattern?

No, it's fine.

Is there a better way of doing this?

Yes, have a look at the difference between .then(…).catch(…) and .then(…, …). If you want to strictly distinguish the success case (continue) from the error case (report this specific problem), passing two callbacks to then is a better idea. That way, the outer handler cannot be triggered by a failure from the success case code, only from failures in the promise it was installed on. In your case:

getUser(userId)
.then(function(user) {
    return chargeCreditCard(user)
    .then(function(chargeId) {
        return saveChargeId(chargeId)
        .then(function(chargeHistoryId) {
            return associateChargeToUsers(chargeHistoryId);
            .then(function(result) {
                return finalFormatting(result);
            }, function(error){
                crashreporter.reportError('chargeHistoryId DB failed', error);
                return 3;
            });
        }, function(error){
            crashreporter.reportError('ChargeId DB failed', error);
            return 2;
        });
    }, function(error){
        crashreporter.reportError('Credit card failed', error);
        return 4;
    });
}, function(error){
    crashreporter.reportError('User DB failed', error);
    return 1;
})
.then(showToUser);

Though you might want to use a generic error handler:

getUser(userId)
.catch(function(error){
    crashreporter.reportError('User DB failed', error);
    throw new Error(1);
})
.then(function(user) {
    return chargeCreditCard(user)
    .catch(function(error){
        crashreporter.reportError('Credit card failed', error);
        throw new Error(4);
    });
})
.then(function(chargeId) {
    return saveChargeId(chargeId);
    .catch(function(error){
        crashreporter.reportError('ChargeId DB failed', error);
        throw new Error(2);
    });
})
.then(function(chargeHistoryId) {
    return associateChargeToUsers(chargeHistoryId);
    .catch(function(error){
        crashreporter.reportError('chargeHistoryId DB failed', error);
        throw new Error(3);
    });
})
.then(function(result) {
    return finalFormatting(result);
}, function(error) {
    return error.message;
})
.then(showToUser);

Here, every then callback does return a promise that rejects with the appropriate error on its own. Ideally every of the called functions already did that, when they don't and you need to append a specific catch to each of them you might want to use a wrapper helper function (maybe as part of the crashreporter?).

function withCrashReporting(fn, msg, n) {
    return function(x) {
        return fn(x)
        .catch(function(error){
            crashreporter.reportError(msg, error);
            throw new Error(n);
        });
    };
}
withCrashReporting(getUser, 'User DB failed', 1)(userId)
.then(withCrashReporting(chargeCreditCard, 'Credit card failed', 4))
.then(withCrashReporting(saveChargeId, 'ChargeId DB failed', 2))
.then(withCrashReporting(associateChargeToUsers, 'chargeHistoryId DB failed', 3))
.then(finalFormatting, function(error) {
    return error.message;
})
.then(showToUser);

The problem that I see with this, is that you can end up in a semi-callback hell.

Nah, it's just an appropriate level of wrapping. In contrast to callback hell, it can be flattened down to a maximum nesting of two, and it always has a return value.

If you absolutely want to avoid nesting and callbacks, use async/await, though that's actually even uglier:

try {
    var user = await getUser(userId);
} catch(error) {
    crashreporter.reportError('User DB failed', error);
    return showToUser(1);
}
try {
    var chargeId = chargeCreditCard(user);
} catch(error) {
    crashreporter.reportError('Credit card failed', error);
    return showToUser(4);
}
try {
    var chargeHistoryId = saveChargeId(chargeId);
} catch(error) {
    crashreporter.reportError('ChargeId DB failed', error);
    return showToUser(2);
}
try {
    var result = associateChargeToUsers(chargeHistoryId);
} catch(error) {
    crashreporter.reportError('chargeHistoryId DB failed', error);
    return showToUser(3);
}
return showToUser(finalFormatting(result));

回答2:

You should just have one catch for your chain, but you can add more context to the errors that are thrown in each function. For example, when an error condition occurs in chargeCreditCard you could tack onto the error a message property corresponding to what you want to report. Then in your catch error handler you can pass that message property into the reporter:

getUser(userId)
  .then(chargeCreditCard)
  .catch(reportError);

function reportError(error) {
  crashreporter.reportError(error.message, error);
}

回答3:

Here is how you can accomplish "cancelling" the chain without ever nesting callbacks. This is why promises are a solution to "callback hell"...

var errors = {
    USER: 'User DB failed',
    CHARGE: 'ChargeId DB failed',
    HISTORY: 'chargeHistoryId DB failed',
};

function onError(type, error) {
    throw {
         message: type,
         error: error
    }
}

getUser(userId)
  .then(chargeCreditCard, onError.bind(null, errors.USER))
  .then(saveChargeId, function (error) {
    if (error.message === errors.USER) throw error
    else onError(errors.CHARGE, error);
  })
  .then(associateChargeToUsers, function(error) {
    if (error.message === errors.CHARGE || error.message === errors.USER) throw error
    else onError(errors.HISTORY, error);
  })
  .then(finalFormatting, function(error) {
    crashreporter.reportError(error.message, error.error); 
  })
  .then(showToUser);

Essentially, if the first promise fails, you pass its error down the chain of failure callbacks and when it reaches the end, where it gets logged. No other promises are created, so you have effectively "cancelled" the operation on first failure.