This issue can't be edited

Because it belongs to an archived project. Jira admins can restore projects from the archive.

Don't log and throw exceptions

Description

Hibernate is logging errors and then throwing exceptions. Here is a sample piece of code:

catch (SQLException sqle) {
JDBCExceptionReporter.logExceptions(sqle);
throw sqle;
}

This is a software antipattern. Either log the error and don't throw the exception, or throw the exception and don't log the error. In my case, I want to catch HibernateException, and handle it myself, but I can't do that without these errors from Hibernate coming through my log. I can turn error off in my log for hibernate.util.JDBCExceptionReporter, but
a) I shouldn't have to
and
b) This causes me to lose errors where logExceptions was used properly, such as this piece of code:

catch (SQLException e) {
//noncritical, swallow and let the other propagate!
JDBCExceptionReporter.logExceptions(e);
}

Environment

None

Activity

Show:

CVS Notification ServiceNovember 18, 2004 at 7:57 PM

CVS COMMIT LOG:
SUBJECT: [Hibernate-commits] Hibernate2/src/net/sf/hibernate/impl SessionImpl.java,1.84,1.85
Don't log and throw, only throw,

CVS Notification ServiceNovember 18, 2004 at 7:35 PM

CVS COMMIT LOG:
SUBJECT: [Hibernate-commits] Hibernate2/src/net/sf/hibernate/impl SessionImpl.java,1.83,1.84
Don't log and throw, only throw,

Christian BauerNovember 12, 2004 at 10:59 PM

I'm actually only removing the log-throw in SessionImpl. You can turn off logging for StaleObjectStateException if you like.

AttilaANovember 12, 2004 at 10:57 PM

To Christian: thank you, I'm glad for this to be fixed. I'll check it out as soon as the anon CVS at SourceForge catches up.

To Tim: the idiom of log-and-rethrow is probably present in more places throughout the codebase, and I think it'd be a bit naive to assume good Hibernate folks could review all of them by now, so the issue is probably far from being fully fixed. I think the best we can do is request remedying concrete occurrences of the problem that affect us when they pop up (expressed more shortly as "ask for scratching where it itches"), preferrably with patches, so the thing can move along incrementally. I.e. if Chris' CVS fix only includes the two occurrences I mentioned here, I'm a happy camper for now.

TimTNovember 12, 2004 at 9:51 PM

Great to hear it! Does that mean you should change the resolution on this bug from "Rejected" to "Fixed"?

Rejected

Details

Assignee

Reporter

Bug Testcase Reminder (view)

Bug reports should generally be accompanied by a test case!

Bug Testcase Reminder (edit)

Bug reports should generally be accompanied by a test case!

Participants

AttilaA
Christian Bauer
CVS Notification Service
Emmanuel Bernard
GavinG
TimT

Components

Affects versions

Priority

Created November 10, 2004 at 11:18 PM
Updated November 18, 2004 at 7:57 PM
Resolved November 11, 2004 at 5:20 AM