[
https://issues.jboss.org/browse/RF-12897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768876#comment-12768876
]
Lukáš Fryč edited comment on RF-12897 at 4/19/13 11:42 AM:
-----------------------------------------------------------
The exception is swallowed here:
https://github.com/richfaces/richfaces5/blob/master/framework/src/main/java/org/richfaces/context/ExtendedPartialViewContextImpl.java#L540
As I can see it, it was a try to improve Mojarra code in order to catch the
exception, but Mojarra now fixed it in a much better way: catches just
{{IOException}} and wraps it in {{FacesException}}. Non-checked exceptions are
then propagated and caught higher in the tree, which is completely right.
We can do what Mojarra does here:
{code:title=Mojarra 2.1.19:
http://java.net/projects/mojarra/sources/svn/content/trunk/jsf-ri/src/main/java/com/sun/faces/context/PartialViewContextImpl.java:548}
public VisitResult visit(VisitContext context, UIComponent comp) {
try {
if (curPhase == PhaseId.APPLY_REQUEST_VALUES) {
// RELEASE_PENDING handle immediate request(s)
// If the user requested an immediate request
// Make sure to set the immediate flag here.
comp.processDecodes(ctx);
} else if (curPhase == PhaseId.PROCESS_VALIDATIONS) {
comp.processValidators(ctx);
} else if (curPhase == PhaseId.UPDATE_MODEL_VALUES) {
comp.processUpdates(ctx);
} else if (curPhase == PhaseId.RENDER_RESPONSE) {
PartialResponseWriter writer =
ctx.getPartialViewContext().getPartialResponseWriter();
writer.startUpdate(comp.getClientId(ctx));
// do the default behavior...
comp.encodeAll(ctx);
writer.endUpdate();
} else {
throw new IllegalStateException("I18N: Unexpected " +
"PhaseId passed to " +
" PhaseAwareContextCallback: " +
curPhase.toString());
}
}
catch (IOException ex) {
if (LOGGER.isLoggable(Level.SEVERE)) {
LOGGER.severe(ex.toString());
}
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE,
ex.toString(),
ex);
}
throw new FacesException(ex);
}
// Once we visit a component, there is no need to visit
// its children, since processDecodes/Validators/Updates and
// encodeAll() already traverse the subtree. We return
// VisitResult.REJECT to supress the subtree visit.
return VisitResult.REJECT;
}
}
{code}
----
I propose to write test which should show that when renderer throws exception
during encoding, the exception should be propagated to the client.
I guess that no update will be actually written in the partial-response, just
view-state.
was (Author: lfryc):
The exception is swallowed here:
https://github.com/richfaces/richfaces5/blob/master/framework/src/main/java/org/richfaces/context/ExtendedPartialViewContextImpl.java#L540
As I can see it, it was a try to improve Mojarra code in order to catch the
exception, but Mojarra now fixed it in a much better way: catches just
{{IOException}} and wraps it in {{FacesException}}. Non-checked exceptions are
then propagated and caught higher in the tree, which is completely right.
We can do what Mojarra does here:
{code:title=Mojarra 2.1.19:
http://java.net/projects/mojarra/sources/svn/content/trunk/jsf-ri/src/main/java/com/sun/faces/context/PartialViewContextImpl.java:548}
public VisitResult visit(VisitContext context, UIComponent comp) {
try {
if (curPhase == PhaseId.APPLY_REQUEST_VALUES) {
// RELEASE_PENDING handle immediate request(s)
// If the user requested an immediate request
// Make sure to set the immediate flag here.
comp.processDecodes(ctx);
} else if (curPhase == PhaseId.PROCESS_VALIDATIONS) {
comp.processValidators(ctx);
} else if (curPhase == PhaseId.UPDATE_MODEL_VALUES) {
comp.processUpdates(ctx);
} else if (curPhase == PhaseId.RENDER_RESPONSE) {
PartialResponseWriter writer =
ctx.getPartialViewContext().getPartialResponseWriter();
writer.startUpdate(comp.getClientId(ctx));
// do the default behavior...
comp.encodeAll(ctx);
writer.endUpdate();
} else {
throw new IllegalStateException("I18N: Unexpected " +
"PhaseId passed to " +
" PhaseAwareContextCallback: " +
curPhase.toString());
}
}
catch (IOException ex) {
if (LOGGER.isLoggable(Level.SEVERE)) {
LOGGER.severe(ex.toString());
}
if (LOGGER.isLoggable(Level.FINE)) {
LOGGER.log(Level.FINE,
ex.toString(),
ex);
}
throw new FacesException(ex);
}
// Once we visit a component, there is no need to visit
// its children, since processDecodes/Validators/Updates and
// encodeAll() already traverse the subtree. We return
// VisitResult.REJECT to supress the subtree visit.
return VisitResult.REJECT;
}
}
{code}
----
I propose to write test which should show that when renderer throws exception
during encoding, the exception should be propagated to the client.
> ExtendedPartialViewContextImpl swallows exceptions
> --------------------------------------------------
>
> Key: RF-12897
> URL: https://issues.jboss.org/browse/RF-12897
> Project: RichFaces
> Issue Type: Bug
> Security Level: Public(Everyone can see)
> Components: core
> Affects Versions: 4.2.2.Final
> Environment: Windows 7, JDK 1.6
> Reporter: Cristian Cerb
> Priority: Critical
> Labels: richfaces
> Fix For: 4.3.2
>
> Original Estimate: 4 hours
> Remaining Estimate: 4 hours
>
> Method public VisitResult visit(VisitContext context, UIComponent target) of
> class ExtendedPartialViewContextImpl swallows exceptions. I think exceptions
> should be wrapped inside a FaceException and propagated. This class
> http://java.net/projects/mojarra/sources/svn/content/trunk/jsf-ri/src/main/java/com/sun/faces/context/PartialViewContextImpl.java
> seems to have served as a model for the RF one, but the RF class
> inadvertently chose to swallow exceptions instead of propagating them up.
> This causes unpredictable behaviour, such as system being stuck in the
> browser.
> I just found an older version of Sun's Mojarra where they had the same issue,
> but the newer versions seem to have fixed it. See this taken from jsf 2.1.4:
> {code}
> public VisitResult visit(VisitContext context, UIComponent comp) {
> try {
> if (curPhase == PhaseId.APPLY_REQUEST_VALUES) {
> // RELEASE_PENDING handle immediate request(s)
> // If the user requested an immediate request
> // Make sure to set the immediate flag here.
> comp.processDecodes(ctx);
> } else if (curPhase == PhaseId.PROCESS_VALIDATIONS) {
> comp.processValidators(ctx);
> } else if (curPhase == PhaseId.UPDATE_MODEL_VALUES) {
> comp.processUpdates(ctx);
> } else if (curPhase == PhaseId.RENDER_RESPONSE) {
> PartialResponseWriter writer =
> ctx.getPartialViewContext().getPartialResponseWriter();
> writer.startUpdate(comp.getClientId(ctx));
> try {
> // do the default behavior...
> comp.encodeAll(ctx);
> }
> catch (Exception ce) {
> if (LOGGER.isLoggable(Level.SEVERE)) {
> LOGGER.severe(ce.toString());
> }
> if (LOGGER.isLoggable(Level.FINE)) {
> LOGGER.log(Level.FINE,
> ce.toString(),
> ce);
> }
> }
> writer.endUpdate();
> }
> else {
> throw new IllegalStateException("I18N: Unexpected " +
> "PhaseId passed to " +
> " PhaseAwareContextCallback: " +
> curPhase.toString());
> }
> }
> catch (IOException ex) {
> ex.printStackTrace();
> }
> // Once we visit a component, there is no need to visit
> // its children, since processDecodes/Validators/Updates and
> // encodeAll() already traverse the subtree. We return
> // VisitResult.REJECT to supress the subtree visit.
> return VisitResult.REJECT;
> }
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
richfaces-issues mailing list
[email protected]
https://lists.jboss.org/mailman/listinfo/richfaces-issues
