db-transaction* doesn't handle .setAutoCommit throwing exception

Description

db-transaction* executes the function it is given within a (try ... (finally ...)), and within finally it calls (.setAutoCommit con auto-commit) as well as (possibly) .setTransactionIsolation and .setReadOnly, in order to restore the connection's original values for those attributes.

Those three methods are documented as being allowed to throw SQLException – including if the connection is closed, which it may now be precisely because of an exception thrown by the function in the (try ... (finally ...)). jTDS, at least, does throw an exception out of (.setAutoCommit ...) when the original exception has caused the connection to be closed. When this happens, the original exception is swallowed in favour of the one thrown by the (.setAutoCommit ...).

The calls within the finally should probably themselves be wrapped within a (try ... (finally ...)), and discarded in favour of allowing the original exception to propagate if the (.setAutoCommit ...) (or other setter invocation) fails.

Environment

None

Activity

Show:

Sean CorfieldJune 1, 2016 at 6:52 PM

This will be in either 0.6.2 or 0.7.0 depending on the outcome of discussions around using clojure.spec in contrib libraries.

Sean CorfieldJune 1, 2016 at 6:51 PM

Ignored exceptions from attempts to restore connection state (since those indicate the connection has gone bad anyway).

Sean CorfieldMay 31, 2016 at 6:34 PM

That's awesome Sam – thank you!

importMay 31, 2016 at 8:04 AM

Comment made by: sam.roberton

I did just manage to reproduce it with the following (using with-db-transaction rather than directly calling db-transaction*, just for convenience):

(clojure.java.jdbc/with-db-transaction [tx {:classname "net.sourceforge.jtds.jdbc.Driver" :subprotocol "jtds:sqlserver" :subname "//<DB-HOST>/<DB-NAME>;socketTimeout=10" :user "<DB-USER>" :password "<DB-PASSWORD>"}] (clojure.java.jdbc/query tx ["waitfor delay '00:00:15'"]))

The exception I get in CIDER is:

Unhandled java.sql.SQLException Invalid state, the Connection object is closed. ConnectionJDBC2.java: 1713 net.sourceforge.jtds.jdbc.ConnectionJDBC2/checkOpen ConnectionJDBC2.java: 2223 net.sourceforge.jtds.jdbc.ConnectionJDBC2/setAutoCommit jdbc.clj: 612 clojure.java.jdbc/db-transaction* jdbc.clj: 618 clojure.java.jdbc/db-transaction* jdbc.clj: 587 clojure.java.jdbc/db-transaction* REPL: 118 user/eval76047 Compiler.java: 6782 clojure.lang.Compiler/eval Compiler.java: 6745 clojure.lang.Compiler/eval core.clj: 3081 clojure.core/eval main.clj: 240 clojure.main/repl/read-eval-print/fn main.clj: 240 clojure.main/repl/read-eval-print main.clj: 258 clojure.main/repl/fn main.clj: 258 clojure.main/repl RestFn.java: 1523 clojure.lang.RestFn/invoke interruptible_eval.clj: 87 clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn AFn.java: 152 clojure.lang.AFn/applyToHelper AFn.java: 144 clojure.lang.AFn/applyTo core.clj: 630 clojure.core/apply core.clj: 1868 clojure.core/with-bindings* RestFn.java: 425 clojure.lang.RestFn/invoke interruptible_eval.clj: 85 clojure.tools.nrepl.middleware.interruptible-eval/evaluate interruptible_eval.clj: 222 clojure.tools.nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn interruptible_eval.clj: 190 clojure.tools.nrepl.middleware.interruptible-eval/run-next/fn AFn.java: 22 clojure.lang.AFn/run ThreadPoolExecutor.java: 1142 java.util.concurrent.ThreadPoolExecutor/runWorker ThreadPoolExecutor.java: 617 java.util.concurrent.ThreadPoolExecutor$Worker/run Thread.java: 745 java.lang.Thread/run

If you call the exact same code, but with with-db-connection instead of with-db-transaction, then the exception you get is a java.net.SocketTimeoutException, which is the behaviour I would expect out of with-db-transaction as well.

I'm not at all familiar with the test cases for clojure.java.jdbc, so I'm not sure whether the above goes any way to helping out get automated coverage for this.

And yes – correct/safe error handling in JDBC is always a real nightmare. I suppose at least in Clojure we have a hope of abstracting that away reasonably, rather than the horrible copy/paste mess that it's so easy for equivalent Java projects to end up wallowing in!

Sean CorfieldMay 31, 2016 at 7:05 AM

Thanks Sam. Do you happen to have repro cases of any of these, which could be added to the test cases?

JDBC seems to be full of unpleasant edge cases disappointed face

Completed

Details

Assignee

Reporter

Priority

Created May 31, 2016 at 6:56 AM
Updated June 1, 2016 at 6:52 PM
Resolved June 1, 2016 at 6:52 PM

Flag notifications