Done adding Catch with return of Promise

[mystikxkemix]

I made it like this for my use. But I didn't know how to do it correctly, that's why I made a pull request. Maybe you can help me doing this

[mystikxkemix]

I made a change in my code, the Catch can retrun a promise of the same type of incoming promise. But there is a issue in one of your test that fail the build. I don't know how you want to handle it.

[ReneB]

Hi @RoryDungan! Since the Promises/A+ spec is pretty hard to read, I'll pose the question: am I right in assuming that this feature is actually required, as per the combination of point 2.2.7.1 and 2.3.2, to be Promises/A+-compatible? It's something I've noticed we actually need in one of our projects, so if this is actually the case, I'm willing to work on this pull request if you can tell me what needs to be done - aside from making it mergeable - to get it approved and added to the library. (As an aside, many thanks for all the hard work on this library, it has helped us out a lot!)

[RoryDungan]

Hey, sorry letting this branch go stale for so long. Reading through the A+ guide it I agree it would seem like this is needed. I just tested and this functionality does work in standard JS promises so would definitely be good to have.

I think in terms of getting this ready to merge again, there are a few things that would need to be done:

• There's a merge conflict that would need to be resolved
• The implementation is only for the generic promise, but would need to be done for the non-generic one too.
• Two tests have been added but none of them actually test that the promise returned by Catch is getting executed or that the value returned from the promise returned by catch is passed to the next Then. I've made some comments on the code explaining this in more detail.

[ReneB]

I've started to work on it a bit in my own fork, but I have a judgment call to make and I wanted to ask you for your opinion.

If we add an overload for Catch, some existing code will become ambiguous. For example:

promise.Catch(e => throw new Exception("This shouldn't happen"));

can no longer be resolved, since - because the return type needs to be implied since there is no return in the throw-only body - both Catch(Func<Exception, IPromise<PromisedT>> onRejected) and Catch(Func<Exception, PromisedT> onRejected) would suffice - and thus the code won't compile anymore, making this a backwards-incompatible change.

OTOH, I could add it as a method with a signature like public IPromise<PromisedT> Recover(Func<Exception, PromisedT> recoveryHandler) and the change would be backwards-compatible, but there would be another method added and the user would need to know just a bit more about how the library is supposed to be used again.

How do you feel about this?

(Edit: the name Recover could, of course, apply equally well to the version with the X(Func<Exception, PromisedT> onRejected) signature because it also, sort of, allows recovery - but it also terminates the chain immediately - and if we were to rename both, we're sort of back at the beginning.)

[ReneB]

Since (I think) I have no way of updating this pull request, see #109 for a new version of the code that should be mergeable and fixes the aforementioned issues.