Menu
▾
▴
Re: [Springframework-developer] OpenSessionInViewFilter - can lose filter-chain exception causes
|
From: Juergen H. <ju...@in...> - 2005-05-02 08:21:47
|
I agree that it makes sense to catch unexpected exceptions in the finally
block too, in particular for code that can be overridden. It still makes
sense to make the SessionFactoryUtils methods that we delegate to by default
as fail-safe as possible too: they are called in all sorts of finally
blocks, within Spring and potentially also in user code.
Juergen
-----Original Message-----
From: spr...@li...
[mailto:spr...@li...]On Behalf
Of Nick Minutello
Sent: Thursday, April 28, 2005 2:17 AM
To: spr...@li...
Subject: Re: [Springframework-developer] OpenSessionInViewFilter - can
lose filter-chain exception causes
Holy crap, these mails took a long time to come through the mail system...
I got all of these 3 mails today. (3 days after they were sent..).
Is there a problem with the SF mailing list / mail server?
> Well, SessionFactoryUtils.closeSessionIfNecessary already catches all
> declared exceptions and logs them at error level, exactly because it is
> usually called in finally blocks, where the original exception should not
> get overridden.
Yes. But your closeSession is overridable - so you dont necessarily
control what gets called there...
In the case I was looking at, it was indeed overridden, and calling
session.flush ... which is where the ISE was coming from...
> Anyway, I've added a catch(RuntimeException) block
> there, logging any RuntimeException at error level too instead of
> propagating it.
Did you put the catch around the filterchain or around the
finally-block? (I looked at viewcvs - couldnt see any change.
Once I looked at my proposed change in an ide, I decided it sucked.
Much simpler is to just put a try-catch around everything in the
finally-block. You always prefer the original exception to propagate
out (if thats why we are in the finally block) - and typically any
exception in cleanup code, you can safely log and continue...
Cheers,
-Nick
On 4/27/05, Juergen Hoeller <ju...@in...> wrote:
> Well, SessionFactoryUtils.closeSessionIfNecessary already catches all
> declared exceptions and logs them at error level, exactly because it is
> usually called in finally blocks, where the original exception should not
> get overridden.
>
> An IllegalStateException is simply unexpected there. I would have expected
> an IllegalStateException thrown by a custom type to be wrapped in a
> HibernateException. Anyway, I've added a catch(RuntimeException) block
> there, logging any RuntimeException at error level too instead of
> propagating it.
>
> Thanks for pointing this out, Nick!
>
> Juergen
>
>
> -----Original Message-----
> From: spr...@li...
> [mailto:spr...@li...]On Behalf
> Of Nick Minutello
> Sent: Monday, April 25, 2005 4:14 AM
> To: spr...@li...
> Subject: [Springframework-developer] OpenSessionInViewFilter - can lose
> filter-chain exception causes
>
> Chaps,
>
> Just having spent some time tracking down a problem (a Hibernate
> Session leak, in fact) - it seems that The OpenSessionInViewFilter is
> hiding some root-cause information that I need.
>
> The problem with try-finally's (ie no catch) is that if we are in the
> finally-block because of an exception, and then we get a new exception
> in the finally block, the original exception will be lost :-o.
>
> In my particular case, the call to closeSession() on line 181 is
> resulting in an IllegalStateException (way deep in a custom type in
> hibernate).
>
> This means that the exception coming out of filterChain.doFilter() on
> line 172 is lost.
> In my particular case the IllegalStateException is occuring as a
> result of the original exception.
>
> 171 try {
> 172 filterChain.doFilter(request, response);
> 173 }
> 174
> 175 finally {
> 176 if (!participate) {
> 177 if (isSingleSession()) {
> 178 // single session mode
> 179 TransactionSynchronizationManager.unbindResource(sessionFactory);
> 180 logger.debug("Closing single Hibernate session in
> OpenSessionInViewFilter");
> 181 closeSession(session, sessionFactory);
> 182 }
> 183 else {
> 184 // deferred close mode
> 185 SessionFactoryUtils.processDeferredClose(sessionFactory);
> 186 }
> 187 }
> 188 }
>
> A better approach is to add a catch block and record any exception
> that comes out of the filter chain - in case we get a new exception in
> the finally bock:
> We then have to make a choice about which we throw out. IMO, the first
> exception is more interesting (but in any case, we throw one and log
> the other- we dont want to lose any information)
>
> Here is what I usually do in this situation:
>
> Exception originalException;
> 171 try {
> 172 filterChain.doFilter(request, response);
> 173 }
> 174
> catch (Exception e) {
> originalException = e;
> }
> 175 finally {
> try {
> 176 if (!participate) {
> 177 if (isSingleSession()) {
> 178 // single session mode
> 179 TransactionSynchronizationManager.unbindResource(sessionFactory);
> 180 logger.debug("Closing single Hibernate session in
> OpenSessionInViewFilter");
> 181 closeSession(session, sessionFactory);
> 182 }
> 183 else {
> 184 // deferred close mode
> 185 SessionFactoryUtils.processDeferredClose(sessionFactory);
> 186 }
> 187 }
> catch (Exception e) {
> if (originalException != null) {
> // yikes we have an exception while cleaning up after
> the first one!
> log.error("Error while cleaning up after exception from
> filter chain", e);
> throw originalException;
> }
> throw e;
> }
> 188 }
>
> In some cases, I would tend towards catching Error as well as Exception.
> Its quite conceivable to get a java.lang.Error - like
> NoSuchMethodError - because of a runtime jar mismatch... its not
> pleasant to have this hidden :-)
> I have had this very problem in a home-grown Hibernate session filter...
>
> Cheers,
>
> -Nick
>
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_ide95&alloc_id396&op=ick
> _______________________________________________
> Springframework-developer mailing list
> Spr...@li...
> https://lists.sourceforge.net/lists/listinfo/springframework-developer
>
> -------------------------------------------------------
> SF.Net email is sponsored by: Tell us your software development plans!
> Take this survey and enter to win a one-year sub to SourceForge.net
> Plus IDC's 2005 look-ahead and a copy of this survey
> Click here to start! http://www.idcswdc.com/cgi-bin/survey?id=105hix
> _______________________________________________
> Springframework-developer mailing list
> Spr...@li...
> https://lists.sourceforge.net/lists/listinfo/springframework-developer
>
-------------------------------------------------------
SF.Net email is sponsored by: Tell us your software development plans!
Take this survey and enter to win a one-year sub to SourceForge.net
Plus IDC's 2005 look-ahead and a copy of this survey
Click here to start! http://www.idcswdc.com/cgi-bin/survey?id5hix
_______________________________________________
Springframework-developer mailing list
Spr...@li...
https://lists.sourceforge.net/lists/listinfo/springframework-developer
|
