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

wrap-exceptions does not throw Exceptions (if wrapped in try+) #67

Closed
jwr opened this issue Apr 25, 2012 · 3 comments
Closed

wrap-exceptions does not throw Exceptions (if wrapped in try+) #67

jwr opened this issue Apr 25, 2012 · 3 comments

Comments

@jwr
Copy link

jwr commented Apr 25, 2012

I just discovered, much to my surprise, that my code that has a try+/catch block with a (catch Exception e...) at the end does not catch "403" exceptions thrown by wrap-exceptions. It does catch everything else, e.g. other exceptions, just not those thrown by wrap-exceptions.

This is at least unintuitive and I'm not quite sure what is the right way to go, but: I filed an issue with slingshot (scgilardi/slingshot#24) and now I'm thinking whether it is a good idea for a library such as clj-http to throw exceptions using throw+.

I think (I'm not 100% certain, though) that while using try+/throw+ within the library is fine, exceptions that do not get caught should follow the standard Java conventions. I did not expect to see throw+ in clj-http and I had to look at the source code to see it.

@dakrone
Copy link
Owner

dakrone commented Apr 25, 2012

I've played with this, so far it seems like a Slingshot issue, I'll talk to Steve and get to the bottom of it.

@dakrone
Copy link
Owner

dakrone commented Apr 25, 2012

I talked to Steve about this (see his comment on the slingshot issue). I think I would rather have a little bit of a disconnect (like having to do (catch Object o ...) or (catch map? m ...) in the try+) since it gives the benefit of being able to encapsulate the data in the response.

If you don't care about the contents of the exception, you could easily do a (catch Exception e ...) with a regular try, and if you do, I would recommend using (catch Object o ...).

Does this work for your usage?

@jwr
Copy link
Author

jwr commented Apr 26, 2012

Please take a look at my comments in the slingshot issue: scgilardi/slingshot#24 (comment)

Basically, I can't find anything wrong with what you're doing, it's just that I did not expect that changing try to try+ would start missing exceptions. It is the combination of slingshot unwrapping behavior and the fact that you are using throw+ that caused the issue.

I can see how it makes sense to throw maps with full information — so as for clj-http I'd just suggest that you change the documentation somewhat. Currently it says:

"clj-http will throw a Slingshot Stone that can be caught by a regular (catch Exception e ...) or in Slingshot's try+ block"

... which is true, but you can't expect both to work, e.g. (try+ (http/get...) (catch Exception e)) will not work.

@dakrone dakrone closed this as completed Apr 7, 2014
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