scgilardi / slingshot

Enhanced try and throw for Clojure leveraging Clojure's capabilities
654 stars 28 forks source link

Non-slingshot exceptions with slingshot causes are being ignored in try+/catch block #52

Open aboytsov opened 9 years ago

aboytsov commented 9 years ago

It looks like slingshot gets a bit confused with chained exceptions, specifically when the outer exception is a regular one, and the cause is the slingshot wrapper:

(try                  ;; to catch those that fell through (but shouldn't have!)
  (try                ;; supposed to catch java.lang.Exception
    (try              ;; will catch and rethrow with cause
      (throw+ :test)
      (catch Throwable e
        (println "rethrowing with cause")
        (throw (Exception. e))))
    (catch Exception e
      (println "caught " (type e) "caused by " (type (.getCause e)))))
  (catch Throwable e
    (println "WAS NOT CAUGHT: " (type e) "caused by " (type (.getCause e)))))

user>
rethrowing with cause
caught  java.lang.Exception caused by  clojure.lang.ExceptionInfo

Here things seem to behave as expected. But consider what happens if the only change I make is replace the second try with try+:

(try                  ;; to catch those that fell through (but shouldn't have!)
  (try+               ;; supposed to catch java.lang.Exception
    (try              ;; will catch and rethrow with cause
      (throw+ :test)
      (catch Throwable e
        (println "rethrowing with cause")
        (throw (Exception. e))))
    (catch Exception e
      (println "caught " (type e) "caused by " (type (.getCause e)))))
  (catch Throwable e
    (println "WAS NOT CAUGHT: " (type e) "caused by " (type (.getCause e)))))

user>
rethrowing with cause
WAS NOT CAUGHT:  java.lang.Exception caused by  clojure.lang.ExceptionInfo

What seems to be happening is that under try+ (catch Exception e block doesn't match anything. I looked at the expanded code and it seems like Slingshot is under the impression that this exception is still a wrapped exception (and it initializes :object &throw-context accordingly) even though the wrapped exception is the cause and the main one is a java.lang.Exception.

Needless to say, this is pretty confusing. Am I missing something?

lkrubner commented 8 years ago

@aboytsov -- this issue comes up a lot, but Slingshot does not match on Exception e. Or at least, it didn't for a long time. For those circumstances where you want Slingshot to catch everything, you match on Object:

  (catch Object o
     (println "caught " (type o)  )))