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

make "catch anything" case clearer with additional selector syntax (Issue 24) #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

scgilardi
Copy link
Owner

at long last, a clear syntax for catching any object thrown by throw or throw+.

(try+
    ...
    (catch any e
      ...))

other syntax symbols could be added. I considered some shorthands for Exception classes (e.g., ex, rte, ex-info), but I didn't see a compelling advantage to those over the class name.

@@ -9,7 +9,7 @@
Throwable objects thrown by throw or throw+;

- specify objects to catch by class name, key-values,
predicate, or arbitrary selector form;
predicate, all, or arbitrary selector form;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean 'any' instead of 'all'?

@pjstadig
Copy link
Contributor

pjstadig commented Oct 8, 2014

Having read through the discussion on the issue, I'm trying to understand the need for this change. TL;DR this seem to just be an alternative way of saying (catch Object e ...) and I'm not sure I see the use in that.

Java

The situation in Java is already a bit confusing. There are essentially three classes of exceptions: Exceptions, RuntimeExceptions, and Errors. However, the conventional wisdom is you don't want to catch RuntimeException or Error. In the case of RuntimeException, it usually represents a problem that must be fixed by changing code, so there isn't really a point in trying to recover from it. In the case of Error it usually represents some VM level issue that you also cannot recover from (thread death, OOME). Therefore, you hardly ever want to catch Throwable (from which all three of these inherit).

The additional wrinkle is that RuntimeException inherits from Exception, so if you want to catch all non-RuntimeExceptions you end up having to do something like:

try {
  // some code
} catch (RuntimeException e) {
  throw e;
} catch (Exception e) {
  // actually do something
}

This confusion from Java already translates to Clojure. Catching Exception will not catch Errors, but it will catch RuntimeExceptions which you most often don't want to handle.

Slingshot

Slingshot extends the try/catch/throw mechanism to allow throwing any object. This adds an additional class of thing you can throw. In order to catch anything that could be thrown you must use (catch Object o ...). This seems acceptable. A possible issue with catching Object is that you will also catch Exceptions, but this is analagous to the Exception/RuntimeException issue. It is possible to precisely catch any kind of object by ordering your catch clauses:

(try+
  ;; some code
  (catch RuntimeException e
    ...)
  (catch Exception e
    ;; Exceptions other than RuntimeException
    )
  (catch Error e
    ...)
  (catch Throwable t
    ;; Throwables other than RuntimeException, Exception, or Error
    ...)
  (catch Object o
    ;; Any thing that is thrown other than something that inherits from Throwable
    ...))

In my reading of the discussion on the issue I've seen three responses to this:

  1. "I didn't know this..." Which is a valid response, and the slingshot documentation has been updated. There's a slight learning curve here, but that's the nature of doing new things and using new libraries. I don't see any further action that should result from this.
  2. "It's ugly to say (catch Object o ...)" If this is a purely asthetic judgement, then I'm not sure there is any way to resolve it. I don't find it ugly. If what this really means is "it's different", well so is (catch [:foo :bar] {:keys [foo] :as e} ...). The point of slingshot is to do something different than a plain try/catch. Again I don't see any further action that can result from this.
  3. "It is confusing to have a class selector that selects against the wrapped object instead of the wrapper." In other words (catch RuntimeException e ...) selects against the wrapper object that is thrown whereas (catch Object o ...) selects against the object that is inside the wrapper object that is thrown. It is true this could be confusing, but it's not entirely true that (catch Object o ...) does not match against the wrapper object (try ... (catch Object o ...)) will catch all exceptions as well as any other non-throwable that is thrown. It is incorrect to think of catching Object as catching any non-throwable that has been thrown. It should be thought of like Throwable, it is the top of the hierarchy and you need to order catch clauses if you want to be more precise.

Conclusion

Having to use Object in a catch clause is no more confusing than the situation in Java. It requires having to understand the situation and change your tactic with try+ blocks. It does mean that slingshot is not a drop in replacement for try blocks, but that's OK as long as that is made clear.

There might be some argument with 3 for adding an 'any' syntax that is not a class selector, but personally I would think that it just adds a bit more of a burden of understanding and a less "ugly" way to catch things than just using Object in a catch clause.

So that's not to say I'm against this change if others really find it useful, but I'm a -0.

As far as the implementation goes it seems fine to me.

@scgilardi scgilardi force-pushed the issue-24 branch 2 times, most recently from bd397fb to 33e7fd1 Compare October 21, 2014 22:52
analogous to catching Throwable in try, you can now (catch any e ...)
@scgilardi scgilardi changed the title Issue 24 make "catch anything" case clearer with additional selector syntax (Issue 24) Oct 28, 2014
@lkrubner
Copy link

I often use "(catch Object o ...)" even if I am catching RuntimeException. Often, with RuntimeException, I want to write something to the log, and then die. But I often want to pass local variables to the log, variables that were active right before the exception. That helps with debugging. And being able to grab local variables, put them in an object, throw the object, and then later catch it and write everything to the log -- that is one of the reasons that I like Slingshot. Slingshot gives me that flexibility.

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

Successfully merging this pull request may close these issues.

3 participants