Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Catch<TException> to IPromise and IPromise<TPromised> interface? #57

Open
sindrijo opened this issue Dec 15, 2017 · 3 comments
Open

Comments

@sindrijo
Copy link
Contributor

sindrijo commented Dec 15, 2017

The only difference from a normal Catch is that if the exception does not match the expected type the onRejected handler is not called.

Code that looks like this:

Blackbox.ReturnsPromise()
  .Catch(ex => 
  {
      var specificException = ex as SpecificExceptionType;
      if(ex != null)
      {
          // Handle specific error
      }

      var otherSpecificException = ex as OtherSpecificExceptionType;
      if(otherSpecificException != null)
      {
          // Handle other specific error
      }

      // We didn't handle any of the specific exception types
  }).Then( data => 
  {
      // Do something useful
  });

Then can be written like this:

Blackbox.ReturnsPromise()
  .Catch<SpecificExceptionType>(ex => 
  {
      // Handle specific error
  })
  .Catch<OtherSpecificExceptionType>(ex => 
  {
      // Handle other specific error
  })
  .Catch(ex => 
  {
      // We didn't handle any of the specific exception types
  })
  .Then( data => 
  {
      // Do something useful
  });

The changes required to support this are small. Here is the implementation for the generic promise type.

public IPromise<PromisedT> Catch<TException>(Action<TException> onRejected) 
where TException : Exception
 {
    var resultPromise = new Promise<PromisedT>();
    resultPromise.WithName(Name);
    Action<PromisedT> resolveHandler = v =>
    {
        resultPromise.Resolve(v);
    };

    Action<Exception> rejectHandler = ex =>
    {
        var specificException = ex as TException;
        if (specificException != null)
        {
            onRejected(specificException);
        }

        resultPromise.Reject(ex);
    };

    ActionHandlers(resultPromise, resolveHandler, rejectHandler);

    return resultPromise;
}
@sindrijo sindrijo changed the title Add Catch<TException> to IPromise<TPromised> interface? Add Catch<TException> to IPromise and IPromise<TPromised> interface? Dec 15, 2017
@RoryDungan
Copy link
Contributor

That's an interesting idea. It deviates from the standard a bit although I think as long as the regular Catch still works the same way we can still be fully standard compliant. This functionality would be impossible in JavaScript anyway because it requires a strong type system.

I think I'd be happy to merge this if you submitted a pull request with the new functionality, although it would require tests and probably a section in the documentation explaining how to use it.

Your example implementation looks good. Initially I wasn't sure how it would handle exceptions of derived types (e.g. you do a Catch<ArgumentException> and the code throws an ArgumentNullException), but it looks like it will catch those, which is good because that's the same as regular try { ... } catch blocks.

@ashleydavis
Copy link
Contributor

+1 I always wanted the promise library to work this way, although the design of this needs careful consideration so that it works in a way that makes sense.

@sindrijo
Copy link
Contributor Author

sindrijo commented Feb 2, 2018

I have been using this in my own fork for a while, though it was before the breaking changes that changed how Catch works, will be working on #61 during the weekend and will see if I get around to including this change as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants