Hey All -
I came across this old thread while dealing with a similar problem. It
doesn't look like we've got this quite right yet, so figured it was
worth following up here.
TL;DR: JSF needs to stop wrapping random/unexpected exceptions in
AbortProcessingExceptions as this leads to exception handling madness.
Let's look at 2 command buttons:
1. <h:commandButton action="#{test.handleAction}"/>
2. <h:commandButton actionListener="#{test.handleActionEvent}"/>
Where the method expressions evaluate to:
> public String handleAction()
> {
> throw new IllegalStateException("Action failed.");
> }
>
> public void handleActionEvent(ActionEvent event)
> {
> throw new IllegalStateException("ActionListener failed.");
> }
Think it's fairly safe to say that our goal should be for these two
exceptions to be handled by JSF in a (roughly) consistent manner.
Anything else is going to:
a) confuse our users, and…
b) make it unnecessarily difficult to write exception handling logic
I haven't tested against MyFaces just yet, but based on my Mojarra 2.0.x
testing, we aren't meeting this goal. In particular:
#1 results in a FacesException (indirectly wrapping the
IllegalStateException) being delivered to the ExceptionHandler. This
triggers the default ExceptionHandler's standard behavior - eg. when
running in development project stage, we get the nice Facelets error
page. (Yay!)
#2 results in an AbortProcessingException (wrapping the
IllegalStateException) being delivered to the ExceptionHandler. This
triggers the following bit of cryptic spec behavior (or non-behavior),
from section 6.2.1, "Default ExceptionHandler implementation":
> Upon encountering the first such Exception that is not an instance of
> javax.faces.event.AbortProcessingException, the corresponding
> ExceptionEvent must be set so that a subsequent call to
> getHandledExceptionEvent() or getHandledExceptionEvents() returns that
> ExceptionEvent instance…
As a result, in case #2 the default ExceptionHandler ignores our
IllegalStateException. No error page. (Boo!)
Although the fact that these identical exceptions result in such
different behavior is the most blatant problem, there is a more subtle
issue lurking: by automatically wrapping all exceptions thrown by
actionListeners in AbortProcessingExceptions, we make it very difficult
for a custom ExceptionHandler to distinguish between:
- "Expected" AbortProcessingExceptions - ie. those that are explicitly
thrown by a listener for control flow purposes.
- "Unexpected" AbortProcessingExceptions - ie. those that are implicitly
thrown by the JSF implementation when an "unexpected" exception occurs
in a listener.
These two cases need to be easily distinguishable. Without this, an
ExceptionHandler cannot possibly know whether it should ignore the
(expected) exception or sound the alarm.
Why does JSF wrap random exceptions thrown by listeners in
AbortProcessingExceptions?
This behavior was introduced in the 1.2, with the introduction of
MethodExpressionActionListener, which says:
> First, try to invoke the MethodExpression passed to the constructor of
> this
> instance, passing the argument as the argument. If a
> MethodNotFoundException
> is thrown, call to the zero argument MethodExpression derived from the
> MethodExpression passed to the constructor of this instance. If that
> fails
> for any reason, throw an AbortProcessingException, including the cause
> of the
> failure.
The mention of AbortProcessingException is this bit of the spec/javadoc
is the root of our problems. Not sure what led to the inclusion of this
in the spec (anyone remember?), but I don't see how this behavior can
possibly be correct. It conflicts with the purpose of
AbortProcessingException as stated by the AbortProcessingException javadoc:
> An exception that may be thrown by event listeners to terminate the
> processing of the current event.
As far as I can tell, AbortProcessingException was always intended for
listener-driven control flow. Kind of similar in spirit to DOM's
event.stopPropagation(), but in exception form. I can see arguments for
wanting to implement similar control flow behavior for random/unexpected
exceptions (ie. possible that we want random exceptions to trigger
similar abort processing behavior). However, it is critical that
users/ExceptionHandlers, including the default ExceptionHandler
implementation, be able to determine whether an exception is unexpected
and thus needs to be handled. That's not (easily) possible at the moment.
The simplest/smallest fix is to remove the AbortProcessingException
references/usages from MethodExpressionActionListener and
MethodExpressionValueChangeListener.
If this isn't possible for any reason, we can get more creative about
how we solve this. However we approach this from an API perspective, our
end goal should be that JSF implementations no longer automatically
promote random/unexpected exceptions to AbortProcessignExceptions.
Oh, and, while we're at it, we should think about more generally
cleaning up EL-based exceptions so that they appear in some
sanitized/sane form when they reach the ExceptionHandler. Even in the
"action" case (#1 above), the exception that ends up being queued to the
ExceptionHandler is confusing (at least in Mojarra):
FacesException --> FacesException --> javax.faces.el.EvaluationException
--> IllegalStateException.
Yikes!
I would love to see both cases #1 and #2 result in a simpler exception
chain, eg:
ELException --> IllegalStateException
And though I am not sure whether we need to spec this, it sure would be
nice for this to be consistent across MyFaces and Mojarra.
Andy
On 7/1/11 9:18 AM, Martin Marinschek wrote:
> We should never - ever - just swallow a RuntimeException coming out of
> a listener call. It should _always_ be passed to the exception
> handler.
>
> We agreed on this on the way to JSF 2.0.
>
> best regards,
>
> Martin
>
> On 7/1/11, Leonardo Uribe <lu4242_at_gmail.com> wrote:
>
>> Hi
>>
>> Checking JSF 2.0 ExceptionHandler API, an user in MyFaces notice there
>> is an inconsistency about how AbortProcessingException is handled
>> (MYFACES-3199).
>>
>> Let's suppose this example:
>>
>> <h:inputText valueChangeListener="#{bean.processValueChange}">
>>
>> The spec javadoc of
>> javax.faces.event.MethodExpressionValueChangeListener.processValueChange
>> (on method detail) says this:
>>
>> "... Call through to the MethodExpression passed in our constructor.
>> First, try to invoke the MethodExpression passed to the constructor of
>> this instance, passing the argument ValueChangeEvent as the argument.
>> If a MethodNotFoundException is thrown, call to the zero argument
>> MethodExpression derived from the MethodExpression passed to the
>> constructor of this instance. If that fails for any reason, throw an
>> AbortProcessingException, including the cause of the failure. ..."
>>
>> Look the last line that says "... If that fails for any reason, throw
>> an AbortProcessingException, including the cause of the failure ...".
>> Now suppose a RuntimeException is thrown inside the listener. The spec
>> clearly says that any exception should be wrapped into an
>> AbortProcessingException, and the final effect is that exception is
>> just logged and the page is rendered, so from the client side you
>> don't see any error.
>>
>> The problematic code is this one:
>>
>> <h:inputText >
>> <f:valueChangeListener binding="#{bean}" />
>> </h:inputText>
>>
>> In this case, if a RuntimeException or any other happens inside the
>> listener, instead catch it and rethrow it as an
>> AbortProcessingException, the exception "comes out", the error page is
>> shown. This is the stack trace in Mojarra:
>>
>> java.lang.RuntimeException: process validation throwValueChange failed!
>> at
>> org.apache.myfaces.testex.UserBean2.processValueChange(UserBean2.java:57)
>> at
>> com.sun.faces.facelets.tag.jsf.core.ValueChangeListenerHandler$LazyValueChangeListener.processValueChange(ValueChangeListenerHandler.java:128)
>> at
>> javax.faces.event.ValueChangeEvent.processListener(ValueChangeEvent.java:134)
>> at
>> javax.faces.component.UIComponentBase.broadcast(UIComponentBase.java:777)
>> at javax.faces.component.UIViewRoot.broadcastEvents(UIViewRoot.java:752)
>> at javax.faces.component.UIViewRoot.processValidators(UIViewRoot.java:1167)
>> at
>> com.sun.faces.lifecycle.ProcessValidationsPhase.execute(ProcessValidationsPhase.java:76)
>> at com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
>> at com.sun.faces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:118)
>> at javax.faces.webapp.FacesServlet.service(FacesServlet.java:508)
>>
>> Since both MyFaces and Mojarra shares the same code from facelets, the
>> effect is present in both implementations.
>>
>> I think the problem can be found too on f:actionListener and other
>> similar tags.
>>
>> It is clear the wrapping should be done and the error just need to be
>> logged, but before apply any fix on 2.0.x or 2.1.x branch I think it
>> is better to ask if that is true. Anyway, a clarification must be
>> included on the spec for JSF 2.2, because it could be components in
>> JSF libraries that requires do this fix too.
>>
>> Suggestions are welcome.
>>
>> Leonardo Uribe
>>
>>
>
>
>