Catching Throwable vs Exception?

[Sean Corfield]

Folks, can I get some input on an issue logged against clojure.java.jdbc:

http://dev.clojure.org/jira/browse/JDBC-11

The current code is shown below but the crux of the issue is that if
an Error occurs during processing of the SQL operation (or
user-supplied expression executed in that context), the library code
still attempts to commit the transaction. That's intuitively wrong but
there is some argument that "all bets are off" in the event of an
Error rather than an Exception. My initial reaction would be to catch
Throwable, set rollback true, and rethrow but looking at the current
handling of Exception, I noticed it wraps the original exception and
throws that... which also feels a bit wrong to me: shouldn't the
original exception be thrown without arbitrary wrapping?

So, two questions to the group:
1) is it reasonable to catch Throwable at all?
2) is it reasonable to wrap Exceptions like that?

I think I'm leaning toward removing the finally (pushing that code
into a function called in both the try body and a catch on Exception),
and letting Throwable escape without attempting to clean up at all.
I'm also leaning toward not wrapping the Exception on transaction
rollback - I don't see any concrete benefit to that (perhaps Stephen
can shed some insight there since he wrote the original code?).

Thanx for any input on this.

Sean

(defn throw-rollback
"Sets rollback and throws a wrapped exception"
[e]
(rollback true)
(throw (Exception. (format "transaction rolled back: %s" (.getMessage e)) e)))

(defn transaction*
"Evaluates func as a transaction on the open database connection. Any
nested transactions are absorbed into the outermost transaction. By
default, all database updates are committed together as a group after
evaluating the outermost body, or rolled back on any uncaught
exception. If rollback is set within scope of the outermost transaction,
the entire transaction will be rolled back rather than committed when
complete."
[func]
(binding [*db* (update-in *db* [:level] inc)]
(if (= (:level *db*) 1)
(let [con (connection*)
auto-commit (.getAutoCommit con)]
(io!
(.setAutoCommit con false)
(try
(func)
(catch Exception e
(throw-rollback e))
(finally
(if (rollback)
(.rollback con)
(.commit con))
(rollback false)
(.setAutoCommit con auto-commit)))))
(func))))

[Stuart Sierra]

1) is it reasonable to catch Throwable at all?

Opinion varies, but those who say "don't catch Throwable" are usually referring to the statically-typed world of the Java language and checked exceptions. In the runtime world of the JVM, you cannot know what type of Throwable a user-provided function will throw. For example, Clojure's `assert` macro throws AssertionError, which is not a subclass of Exception.
 

2) is it reasonable to wrap Exceptions like that?

Yes. Java libraries do it all over the place. As long as you don't "swallow" the original exception, you can wrap it in another exception.
 
-Stuart Sierra
clojure.com

[Shantanu Kumar]

2) is it reasonable to wrap Exceptions like that?

Yes. Java libraries do it all over the place. As long as you don't "swallow" the original exception, you can wrap it in another exception.

I feel wrapping of throwables should be done to add some value (e.g. wrapping in a custom exception, which is what the Java libs do) -- otherwise just re-throw the original Throwable. Personally, I would not encourage java.lang.Exception wrappers (makes debugging a lengthier process due to a long wrapper trace.)

Regards

--
Shantanu Kumar || kumar.shantanu(at)gmail || http://twitter.com/kumarshantanu || Skype: shantanu_k06 || +91 99728 69200 (Bangalore)

[Chris Turner]

In general, I would avoid catching Throwable. The canonical reason being: What do you do when you catch an OutOfMemoryError. Sometimes you can do something with it (IntelliJ IDEA, for instance, shows a dialog box allowing you to change startup memory settings before exiting after a OOM error), but most of the time there's nothing you can do other than exit. In fact, in the docs for Error, it specifically states:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions. The ThreadDeath error, though a "normal" condition, is also a subclass of Error because most applications should not try to catch it.

Also, here's an interesting article on a problem that occurred in an app that was catching Throwable

http://www.javatuning.com/why-catch-throwable-is-evil-real-life-story/

Chris

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To post to this group, send email to cloju...@googlegroups.com.
To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.

--
"An ideal world is left as an exercise to the reader."
  - Paul Graham, On Lisp

[Sebastián Galkin]

I think the discussion is centered in catching, but no catching should take place. The logic should be (simplified)

(try

  (func)

  (when-not (rollback? con)

     (.commit con))

  (finally

     (.rollback con)))

I guess it's OK to rollback a commited connection, but didn't tried it.