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 })

Solution 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);
                return3;
            });
        }, function(error){
            crashreporter.reportError('ChargeId DB failed', error);
            return2;
        });
    }, function(error){
        crashreporter.reportError('Credit card failed', error);
        return4;
    });
}, function(error){
    crashreporter.reportError('User DB failed', error);
    return1;
})
.then(showToUser);

Though you might want to use a generic error handler:

getUser(userId)
.catch(function(error){
    crashreporter.reportError('User DB failed', error);
    thrownewError(1);
})
.then(function(user) {
    returnchargeCreditCard(user)
    .catch(function(error){
        crashreporter.reportError('Credit card failed', error);
        thrownewError(4);
    });
})
.then(function(chargeId) {
    returnsaveChargeId(chargeId);
    .catch(function(error){
        crashreporter.reportError('ChargeId DB failed', error);
        thrownewError(2);
    });
})
.then(function(chargeHistoryId) {
    returnassociateChargeToUsers(chargeHistoryId);
    .catch(function(error){
        crashreporter.reportError('chargeHistoryId DB failed', error);
        thrownewError(3);
    });
})
.then(function(result) {
    returnfinalFormatting(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?).

functionwithCrashReporting(fn, msg, n) {
    returnfunction(x) {
        returnfn(x)
        .catch(function(error){
            crashreporter.reportError(msg, error);
            thrownewError(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) {
    returnerror.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));

Solution 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);
}

Solution 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',
};

functiononError(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 errorelse onError(errors.CHARGE, error);
  })
  .then(associateChargeToUsers, function(error) {
    if (error.message === errors.CHARGE || error.message === errors.USER) throw errorelse 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.