Exception handling changes in Clojure 1.3.0

[Constantine Vetoshev]

I ran into an interesting problem while porting appengine-magic to
Clojure 1.3.0.

The Google App Engine SDK uses checked exceptions on many of its API
methods. In many cases, I want to catch these exceptions and do
something Clojure-friendly with them. With Clojure 1.2.x, I had no
trouble catching checked exceptions by type, e.g.:

(try
(some-app-engine-api-methods-called-here ...)
(catch EntityNotFoundException ex ...))

This stopped working in 1.3.0. The caught exception does not match
EntityNotFoundException; it is now a RuntimeException with the
original typed exception chained to it.

I don't fully understand the implications of the exception handling
changes in 1.3 (https://github.com/clojure/clojure/commit/
8fda34e4c77cac079b711da59d5fe49b74605553). Does it mean that all
exceptions coming in from Java code into Clojure will now be wrapped
in an extra RuntimeException?

If so, then typed catch clauses become useless, and the equivalent
functionality will require writing code like this:

(catch Exception ex
(cond (isa? (class (.getCause ex)) EntityNotFoundException) ...))

I can wrap that in some kind of a catch* macro (which would also
support directly catching an exception for Clojure 1.2.x
compatibility), but I'm wondering if I'm doing something wrong here.

[Stuart Halloway]

The Google App Engine SDK uses checked exceptions on many of its API
methods. In many cases, I want to catch these exceptions and do
something Clojure-friendly with them. With Clojure 1.2.x, I had no
trouble catching checked exceptions by type, e.g.:

(try
 (some-app-engine-api-methods-called-here ...)
 (catch EntityNotFoundException ex ...))

This stopped working in 1.3.0. The caught exception does not match
EntityNotFoundException; it is now a RuntimeException with the
original typed exception chained to it.

I don't fully understand the implications of the exception handling
changes in 1.3 (https://github.com/clojure/clojure/commit/
8fda34e4c77cac079b711da59d5fe49b74605553). Does it mean that all
exceptions coming in from Java code into Clojure will now be wrapped
in an extra RuntimeException?

Catching checked exceptions seems to work fine. Try e.g.

(try (throw (java.io.IOException.)) (catch java.io.IOException _ "caught!"))

I suspect something else is going wrong in the GAE example. Can you narrow the code down to a block you can quote in full here?

Stu

Stuart Halloway

Clojure/core
http://clojure.com

[Kevin Downey]

Reflector.java wraps checked exceptions in runtime exceptions

> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clo...@googlegroups.com
> Note that posts from new members are moderated - please be patient with your
> first post.
> To unsubscribe from this group, send email to
> clojure+u...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en

--
And what is good, Phaedrus,
And what is not good—
Need we ask anyone to tell us these things?

[Paul Mooser]

I believe the actual problem comes from things like RT.classForName(),
which internally catches a ClassNotFoundException, and then does this:

throw Util.runtimeException(e);

That ends up just sort of obscuring what the exception is, and you
can't just catch ClassNotFoundException - you have to catch the
RuntimeException, and then figure out what it really means if you
actually have to respond to a specific case. I'm not a particular fan
of checked exceptions, but I don't entirely understand the rationale
behind this.

[Sean Corfield]

On Mon, Oct 3, 2011 at 12:03 PM, Constantine Vetoshev
<gepa...@gmail.com> wrote:
> This stopped working in 1.3.0. The caught exception does not match
> EntityNotFoundException; it is now a RuntimeException with the
> original typed exception chained to it.

This caused me some pain in clojure.java.jdbc and I actually catch
RuntimeException inside the library and unwrap it and throw the
underlying SQLException (for transactions):

;; This ugliness makes it easier to catch SQLException objects
;; rather than something wrapped in a RuntimeException which
;; can really obscure your code when working with JDBC from
;; Clojure... :(
(letfn [(throw-non-rte [ex]
(cond (instance? java.sql.SQLException ex) (throw ex)
(and (instance? RuntimeException ex)
(.getCause ex)) (throw-non-rte (.getCause ex))
:else (throw ex)))]
(throw-non-rte e)))

I've had to do similar things in my own code to get at the underlying
exceptions...

It would be much, much friendlier to have (try .. (catch ..))
automatically unroll RuntimeException "on demand" so the "obvious"
code works.
--
Sean A Corfield -- (904) 302-SEAN
An Architect's View -- http://corfield.org/
World Singles, LLC. -- http://worldsingles.com/
Railo Technologies, Inc. -- http://www.getrailo.com/

"Perfection is the enemy of the good."
-- Gustave Flaubert, French realist novelist (1821-1880)

[Constantine Vetoshev]

On Oct 3, 12:27 pm, Stuart Halloway <stuart.hallo...@gmail.com> wrote:

> Catching checked exceptions seems to work fine. Try e.g.
>
> (try (throw (java.io.IOException.)) (catch java.io.IOException _ "caught!"))
>
> I suspect something else is going wrong in the GAE example. Can you
> narrow the code down to a block you can quote in full here?

Your example works because it doesn't trigger the use of
clojure.lang.Reflector. I'm having a hard time isolating a good
example where an exception passes through Reflector on its way from
Java into Clojure. (It doesn't happen when trivially calling simple
methods on simple Java classes.)

Here is the relevant stack trace, however. The top level invocation of
DatastoreServiceImpl.get throws the exception (level 0), and I hope to
catch it in retrieve (level 9).

0:
com.google.appengine.api.datastore.DatastoreServiceImpl.get(DatastoreServiceImpl.java:
64)
1: sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
2:
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:
39)
3:
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:
25)
4: java.lang.reflect.Method.invoke(Method.java:597)
5: clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:92)
6: clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:30)
7: appengine_magic.services.datastore
$retrieve_helper.doInvoke(datastore.clj:363)
8: clojure.lang.RestFn.invoke(RestFn.java:521)
9: appengine_magic.services.datastore
$retrieve.doInvoke(datastore.clj:376)

[Rok Lenarcic]

On Oct 3, 9:27 pm, Stuart Halloway <stuart.hallo...@gmail.com> wrote:
>
> Catching checked exceptions seems to work fine. Try e.g.
>
> (try (throw (java.io.IOException.)) (catch java.io.IOException _ "caught!"))
>
> I suspect something else is going wrong in the GAE example. Can you narrow the code down to a block you can quote in full here?
>
> Stu
>
> Stuart Halloway
> Clojure/corehttp://clojure.com

Does this work across function boundaries?
(try (exception-throwing-fn 1 2 3) (catch java.io.IOException _
"caught!"))

[Meikel Brandmeyer (kotarak)]

It does.

user=> (defn f [] (Class/forName "nonexistant"))
#'user/f
user=> (try (f) (catch ClassNotFoundException e "caught!"))
"caught!"

[Kevin Downey]

the problem is in Reflector.java and the call to Class/forName is
non-reflective, so of course it doesn't expose the problem.

[Constantine Vetoshev]

I finally came up with a simple example which shows the broken
exception handling behavior I described in my earlier post on this
thread.

(defn broken-catch [filename]
(try (java.io.FileReader. filename)
(catch java.io.FileNotFoundException fnfe
"FileNotFoundException caught")
(catch RuntimeException ioe
(str "RuntimeException caught; cause: "
(.getCause ioe)))))

user=> (broken-catch "/etc/passwdXYZ")
"RuntimeException caught; cause: java.io.FileNotFoundException: /etc/
passwdXYZ (No such file or directory)"

The FileReader constructor throws a FileNotFoundException when the
file specified by the argument does not exist:
http://download.oracle.com/javase/6/docs/api/java/io/FileReader.html#FileReader(java.lang.String)
— and as you can see here, Clojure 1.3.0 wrapped it in a
RuntimeException, so the expected typed catch does not work.

However, if invoked in a way which bypasses reflection, the exception
is properly typed:

user=> (try (java.io.FileReader. "/etc/passwdXYZ")
(catch java.io.FileNotFoundException fnfe
"FileNotFoundException caught")
(catch RuntimeException ioe
(str "RuntimeException caught; cause: "
(.getCause ioe))))
"FileNotFoundException caught"

[Sean Corfield]

Would a good solution to this be for try/catch to _automatically_
unroll RTE if there's no matching Exception class specified?

I understand _why_ the change to wrap exceptions was made but without
the equivalent unwrapping in try/catch this just moves the pain from
the library/language out to the users, yes?

Sean

[Stuart Sierra]

I was a little worried about this when the exception behavior for fns was changed. I think it's solvable, but don't know right now what the solution is.

-S

[pmbauer]

Would now be a bad time to observe there was a large change made to the way exceptions are handled, and the lack of guard-rails (no tests for exception handling behavior) may have contributed to this regression?

https://github.com/clojure/clojure/commit/8fda34e4c77cac079b711da59d5fe49b74605553

[Chas Emerick]

…or that there's many thousands of tests (or many, many, many thousands of tests if you count all the 3rd party libraries that have been testing against 1.3.0 snapshots for months with nary a related hiccup), many of them related to exceptions and other error conditions, but no one was clever enough to write a test ahead of time to account for the change in behaviour. Either way. :-)

Of course, this is how most regression tests are born…

- Chas

[pmbauer]

Fair enough. :) Complete test coverage is intractable.
But in clojure, there are only a handful of tests that contain a catch and none that test try/catch semantics. Maybe I'm looking in the wrong place...

[Rich Hickey]

It's not a regression. It is a known breaking change.

The change merely owned up the the fact that we cannot in fact communicate all actual errors through Java code, due to checked exceptions. This is an enduring problem with Java, and you are going to see it rear its ugly head in Java itself as soon as Java gets some closure-like construct.

We can of course add some unwrapping to try/catch, and could have for 1.3, had anyone who currently cares about this tried the code and acted sooner. Patch welcome for 1.4. Note the handling will have to be careful not to unwrap too much, as some people care about cause chains and not just root causes.

Rich

[pmbauer]

I don't know what the right solution is either, but here is a JIRA ticket with an attached regression test.  A start.

http://dev.clojure.org/jira/browse/CLJ-855

On Tuesday, October 11, 2011 3:02:47 PM UTC-7, Stuart Sierra wrote:

[Ivan Koblik]

I wonder, would it be possible to throw a custom exception (something like DoNotUse_InternalRuntimeException) in Reflector.java, and update try/catch code to always unwrap it?

Cheers,
Ivan.

--

[Steve Miner]

I've done something like this in Java projects but without any magic unwrapping. It worked well.  Manual unwrapping wasn't too onerous in the rare cases where we wanted to do so.  The Clojure-specific exception should be part of the public API for Java interop. Don't try to hide it.

[Meikel Brandmeyer (kotarak)]

Hi,

Am Mittwoch, 12. Oktober 2011 15:19:21 UTC+2 schrieb miner:

I've done something like this in Java projects but without any magic unwrapping. It worked well.  Manual unwrapping wasn't too onerous in the rare cases where we wanted to do so.  The Clojure-specific exception should be part of the public API for Java interop. Don't try to hide it.

Yes. You might call into Clojure from Java-side, where you don't have a magic unwrap-try-catch.

Sincerely
Meikel
 

[Stefan Kamphausen]

Hi,

To my humble ears this sounds like the best idea so far.  Something like ClojureRTException ...

Regards,
Stefan

[Sean Corfield]

On Wed, Oct 12, 2011 at 7:37 AM, Stefan Kamphausen
<ska...@googlemail.com> wrote:
> To my humble ears this sounds like the best idea so far.  Something like
> ClojureRTException ...

The problem - in Clojure code using try/catch - is that you don't know
whether the real exception will be wrapped or not (because you may be
calling thru some intermediate layer that may or may not do
reflection) therefore you cannot usefully catch a given exception in
Clojure: you will _always_ have to catch both the intended exception
*and* catch ClojureRTException and unroll it yourself with the same
code in both branches of logic. That's _horrible_.

try/catch should do the unrolling for you - esp. if we introduce
ClojureRTE as a wrap-only exception so it won't be confused with a
regular RTE.

[Chas Emerick]

On Oct 12, 2011, at 1:21 PM, Sean Corfield wrote:

> On Wed, Oct 12, 2011 at 7:37 AM, Stefan Kamphausen
> <ska...@googlemail.com> wrote:
>> To my humble ears this sounds like the best idea so far. Something like
>> ClojureRTException ...
>
> The problem - in Clojure code using try/catch - is that you don't know
> whether the real exception will be wrapped or not (because you may be
> calling thru some intermediate layer that may or may not do
> reflection) therefore you cannot usefully catch a given exception in
> Clojure: you will _always_ have to catch both the intended exception
> *and* catch ClojureRTException and unroll it yourself with the same
> code in both branches of logic. That's _horrible_.
>
> try/catch should do the unrolling for you - esp. if we introduce
> ClojureRTE as a wrap-only exception so it won't be confused with a
> regular RTE.

I had read the idea as implying that try/catch would implicitly always catch RTE, unwrap it, and give your "real" catch body a chance to catch that…

- Chas

[Marshall T. Vandegrift]

Stuart Sierra <the.stua...@gmail.com> writes:

I'm not incredibly experienced with Java, but would using a Java
construct which tricks the language into allowing unchecked exceptions
be too crazy? I've seen references to both of these approaches, but
haven't actually tried either for anything:

http://james-iry.blogspot.com/2010/08/on-removing-java-checked-exceptions-by.html
http://projectlombok.org/features/SneakyThrows.html

The former even came up on #clojure, I believe.

-Marshall

[Stefan Kamphausen]

Just for the record: That's how I understood Ivan's idea, too.  Introduce a special exception type which is used nowhere else and unwrap that automatically.

I am not experienced enough a Java programmer to know all the implications that may arise here and there.

Regards,
Stefan

[Ben Smith-Mannschott]

I've attached a RFC patch based on this idea to CLJ-855.

http://dev.clojure.org/jira/browse/CLJ-855

// Ben

[Kevin Downey]

On Wed, Oct 12, 2011 at 12:00 PM, Marshall T. Vandegrift
<lla...@gmail.com> wrote:
> Stuart Sierra <the.stua...@gmail.com> writes:
>
>> I was a little worried about this when the exception behavior for fns was
>> changed. I think it's solvable, but don't know right now what the solution
>> is.
>
> I'm not incredibly experienced with Java, but would using a Java
> construct which tricks the language into allowing unchecked exceptions
> be too crazy?  I've seen references to both of these approaches, but
> haven't actually tried either for anything:
>
>  http://james-iry.blogspot.com/2010/08/on-removing-java-checked-exceptions-by.html

seems like refactoring Reflector.java to rethrow using this approach
is preferable to some wrapping/unwrapping scheme

>  http://projectlombok.org/features/SneakyThrows.html
>
> The former even came up on #clojure, I believe.
>
> -Marshall
>

> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clo...@googlegroups.com
> Note that posts from new members are moderated - please be patient with your first post.
> To unsubscribe from this group, send email to
> clojure+u...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en

--