dev@javaserverfaces.java.net

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

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 08 Nov 2005 14:30:35 -0800

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
>
>
>