[Hibernate-issues] [Hibernate-JIRA] Commented: (HB-106) Minor flaw in suggested error handling

[<leg...@at...>]

The following comment has been added to this issue:

     Author: Paul Rivers
    Created: Sun, 25 May 2003 5:13 PM
       Body:
After further experimentation and thought, I realized the entire catch block was rather inconvenient. The original, again, was:

catch (Exception e) { 
   if (tx!=null) tx.rollback(); 
   throw e; 
} 
finally { 
   sess.close(); 
}

However, catching a generic exception e then rethrowing it forces the function that contains the code to throw an "Exception" exception (public void functionName() throws Exception {}). (The same things happens if you're catching Throwable) This is obviously not desirable. Of course, you can catch the exact exceptions that could be thrown, but 1. This gets ugly when their are many exceptions 2. You still have to remember to catch RuntimeExeption. 3. You have to customize what it essentially boiler-plate code each time its used.

Instead, the code can be written so that the outline never needs to be changed, and will correctly handle all types of exceptions if it's written like this:

Session sess = factory.openSession();
Transaction tx = null;
try {
   tx = sess.beginTransaction();
   // do some work
   //...
   tx.commit();
}
finally {
   if(tx != null && !tx.wasCommitted() && !tx.wasRolledBack()) tx.rollback();
   sess.close();
}

(I'm not sure if calling Transaction.rollback() twice could possibly throw an exception, if it can't then the last conditional in the if statement could be removed).
---------------------------------------------------------------------
View the issue:

  http://opensource.atlassian.com/projects/hibernate/secure/ViewIssue.jspa?key=HB-106


Here is an overview of the issue:
---------------------------------------------------------------------
        Key: HB-106
    Summary: Minor flaw in suggested error handling
       Type: Bug

     Status: Unassigned
   Priority: Trivial

    Project: Hibernate2
   Versions:
             2.0rc2

   Assignee: 
   Reporter: Paul Rivers

    Created: Sun, 25 May 2003 4:42 PM
    Updated: Sun, 25 May 2003 4:42 PM
Environment: irrelevant

Description:
I wasn't sure where to put a documentation issue, I thought this would be the best place. Please let me know if this is incorrect, and I'll post elsewhere in the future.

In "6.9.4. Exception handling" in the documentation, the first exception handling idiom is recommended:

Session sess = factory.openSession();
Transaction tx = null;
try {
    tx = sess.beginTransaction();
    // do some work
    ...
    tx.commit();
}
catch (Exception e) {
    if (tx!=null) tx.rollback();
    throw e;
}
finally {
    sess.close();
}

However, notice the "catch (Exception e) {" line. If a RuntimeException is thrown, the catch block will never be entered, and the transaction will never be terminated. Thus that line should be replaced by "catch (Throwable e) {" so that the entire example will read:

Session sess = factory.openSession();
Transaction tx = null;
try {
    tx = sess.beginTransaction();
    // do some work
    ...
    tx.commit();
}
catch (Throwable e) {
    if (tx!=null) tx.rollback();
    throw e;
}
finally {
    sess.close();
}

This also applies to the next 2 examples in the documentation.


---------------------------------------------------------------------
JIRA INFORMATION:
This message is automatically generated by JIRA.

If you think it was sent incorrectly contact one of the administrators:
   http://opensource.atlassian.com/projects/hibernate/Administrators.jspa

If you want more information on JIRA, or have a bug to report see:
   http://www.atlassian.com/software/jira