java.jdbc

in db-transaction* an error in rollback can obscure an application error

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

At https://github.com/clojure/java.jdbc/blob/master/src/main/clojure/clojure/java/jdbc.clj#L719 we have

(try
  (let [result (func nested-db)]
    (if (db-is-rollback-only nested-db)
      (.rollback con)
      (.commit con))
    result)
  (catch Throwable t
    (.rollback con)

If (.rollback con) throws an error, we never throw t. We could wrap the rollback call in a try-finally and throw t in the finally clause, but of course then we'd never find out the rollback failed. May make sense to throw some custom exception wrapping both errors.

Activity

Hide
Sean Corfield added a comment -

I think for that edge case throwing ex-info would be acceptable? Possibly with t set as the cause if ex-info will let me.

Show
Sean Corfield added a comment - I think for that edge case throwing ex-info would be acceptable? Possibly with t set as the cause if ex-info will let me.
Hide
Sean Corfield added a comment -

You can't call .setCause on an ex-info so the combined exception has :rollback and :handling keys, and the message includes the transaction exception message.

Show
Sean Corfield added a comment - You can't call .setCause on an ex-info so the combined exception has :rollback and :handling keys, and the message includes the transaction exception message.
Hide
Sean Corfield added a comment -

Will be in 0.7.3 (which should be on Maven shortly).

Show
Sean Corfield added a comment - Will be in 0.7.3 (which should be on Maven shortly).
Hide
Michael Blume added a comment -

Awesome, thanks =)

Show
Michael Blume added a comment - Awesome, thanks =)

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: