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

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

Open
aboytsov opened this issue Jul 3, 2015 · 1 comment

Comments

@aboytsov
Copy link

aboytsov commented Jul 3, 2015

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
Copy link

@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)  )))

rbrw added a commit to rbrw/trapperkeeper that referenced this issue Feb 25, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Feb 26, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Feb 26, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Feb 26, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Feb 26, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Mar 6, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Mar 6, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
rbrw added a commit to rbrw/trapperkeeper that referenced this issue Mar 6, 2020
While we're reworking this code, remove slingshot.  It's easy to
handle the ex-data directly, and then we don't have to keep
slingshot's potentially surprising behaviors in mind, e.g.

  (try+
    ...
    (catch Exception ex
      ...))

may not actually catch ex.  cf.

  scgilardi/slingshot#24
  scgilardi/slingshot#52
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

2 participants