dev@javaserverfaces.java.net

Re: [REVIEW] Ensure ViewExpiredException isn't wrapped when detected

From: Jacob Hookom <jacob_at_hookom.net>
Date: Tue, 08 Nov 2005 17:35:08 -0600

I wasn't sure about the unwinding... the finally clause expectations are
okay, but a FacesException is a FacesException-- including all extended
instances.

Along the lines of the stack trace, if you just re-throw everything
that's a FacesException, then the unwinding becomes less necessary since
we are now avoiding the
FacesException->ValidationException->NullPointerException where
ValidationException is a FacesException and we would just throw
ValidationException.

I'm not familiar with the complete context of the current logic, but
unwinding FacesExceptions instead of re-throwing them seems unecessary.

-- Jacob

Ryan Lubke wrote:

> jacob_at_hookom.net wrote:
>
>> Why don't we setup the logic to say:
>>
>> catch(Exception e) {
>> if (e instanceof FacesException) {
>> throw e;
>> } else {
>> throw new FacesException(e);
>> }
>> }
>>
>> Then you are covered for all expected FacesException types, one being
>> ViewExpiredException.
>>
>>
> I think that change goes against what the finally block logic is trying
> to do. As the inline comment says, allow all the afterPhase listeners
> to execute before throwing the exception caught during phase execution.
>
> Also, the specific handling of ViewExpiredException was intentional.
> In all
> other exception type cases, I wanted to unwind the exception chain first
> be it a standard FacesException or not before wrapping.
>
> Maybe I missed the point of your response?
>
>> -- Jacob
>>
>> Ryan Lubke <Ryan.Lubke_at_sun.com> wrote on 11/08/2005, 10:01:08 PM:
>>
>>
>>> With the current RI implementation, a developer currently can't
>>> add the following to their web.xml:
>>>
>>>
>>> javax.faces.application.ViewExpiredException
>>> /error.jsp
>>>
>>>
>>> and expect the error page to be triggered since the Lifecycle
>>> implementation
>>> wraps all caught exceptions as a FacesException.
>>>
>>> Modify the lifecycle to rethrow ViewExpiredExceptions when found.
>>>
>>> Tested with a simple app locally to confirm that the error page is
>>> now triggered.
>>>
>>>
>>> SECTION: Modified Files
>>> ----------------------------
>>> M src/com/sun/faces/lifecycle/LifecycleImpl.java
>>> - if 'ex' is a ViewExpiredException, rethrow
>>> - for the other case, unwind the exception chain
>>> and throw a new FacesException with the root
>>> cause of the chain
>>>
>>>
>>> SECTION: Diffs
>>> ----------------------------
>>> Index: src/com/sun/faces/lifecycle/LifecycleImpl.java
>>> ===================================================================
>>> RCS file:
>>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/lifecycle/LifecycleImpl.java,v
>>>
>>> retrieving revision 1.56
>>> diff -u -r1.56 LifecycleImpl.java
>>> --- src/com/sun/faces/lifecycle/LifecycleImpl.java 26 Aug 2005
>>> 15:27:08 -0000 1.56
>>> +++ src/com/sun/faces/lifecycle/LifecycleImpl.java 8 Nov 2005
>>> 20:23:37 -0000
>>> @@ -34,6 +34,7 @@
>>> import java.util.logging.Level;
>>>
>>> import javax.faces.FacesException;
>>> +import javax.faces.application.ViewExpiredException;
>>> import javax.faces.context.FacesContext;
>>> import javax.faces.event.PhaseEvent;
>>> import javax.faces.event.PhaseId;
>>> @@ -247,7 +248,7 @@
>>> }
>>> }
>>> }
>>> - catch (Throwable e) {
>>> + catch (Exception e) {
>>> if (logger.isLoggable(Level.WARNING)) {
>>> logger.warning("phase(" + phaseId.toString() + "," +
>>> context +
>>> ") threw exception: " + e + " " + e.getMessage() +
>>> @@ -295,7 +296,14 @@
>>> // Allow all afterPhase listeners to execute before throwing
>>> the
>>> // exception caught during the phase execution.
>>> if (exceptionThrown) {
>>> - throw new FacesException(ex.getCause());
>>> + if (ex instanceof ViewExpiredException) {
>>> + throw (ViewExpiredException) ex;
>>> + }
>>> + // unwind exceptions to root cause
>>> + while (ex.getCause() != null) {
>>> + ex = ex.getCause();
>>> + }
>>> + throw new FacesException(ex);
>>> }
>>> }
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>
>


-- 
Jacob Hookom - Minneapolis
--------------------------
http://hookom.blogspot.com