Skip to content
New issue

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

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

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants