MichaelDrogalis / dire

Erlang-style supervisor error handling for Clojure
483 stars 19 forks source link

Slingshot handler support #17

Closed dparis closed 10 years ago

dparis commented 10 years ago

Resubmitting PR from #16, this time against the develop branch.

All changes agreed upon in #16 have been applied. Sorry I didn't have time to mess around with rebasing my commits, I didn't want to deal with messing things up on GH tonight.

I made the change to throw+ a custom exception from the catch clause within the predicate selector application multimethod. There's a new test case for that circumstance. Works great, and necessary I think, since once I made the chance it broke a couple of other tests where the predicate application was causing an exception and it was being masked entirely. Going forward, it may be a good idea to write up some caveats about using predicate matchers, since they need to be carefully designed to handle pretty much any object type.

I actually think that's a pretty compelling reason to continue working towards being able to specify a priority for matchers. Having more control over the order selectors are applied would allow you to maneuver around predicate selectors, etc, in cases where you didn't want to write them to handle everything under the sun.

For posterity:

  1. We still have yet to work out a way to specify the priority of selector matching.
  2. There may still be some unintuitive behavior when throwing exceptions from within exception handlers. I haven't had time to look into this, but I'm hoping to later in the week since it affects the project I'm working on.

Keep me posted about what you'd like to do next @MichaelDrogalis.

MichaelDrogalis commented 10 years ago

@dparis Thanks for this! No worry about rebasing at all. I will get some docs written up tonight and a blog post out for tomorrow to promote your patch!

Do you want to get in touch over email to discuss the remaining issues? I understand why it's a problem, and we can fast-track those changes. mjd3089 at rit dot edu

MichaelDrogalis commented 10 years ago

This has been merged in, and is available on 0.5.0-SNAPSHOT.

So awesome.

dparis commented 10 years ago

Sure, let's email. Got a busy week ahead of me, but I should have some time towards the end to hack on this a bit more.