myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Koci <martin.kocicak.k...@gmail.com>
Subject Re: [core] three issues with ViewExpiredException & navigation
Date Wed, 29 Jun 2011 18:13:05 GMT
Leonardo Uribe píše v Út 28. 06. 2011 v 16:11 -0500:
> Hi
> 
> 2011/6/28 Martin Koci <martin.kocicak.koci@gmail.com>:
> > Hi,
> >
> > to make things not so easy:
> > https://issues.apache.org/jira/browse/MYFACES-3191
> > Maybe it is related, maybe not, but now it is not possible to perform
> > navigation in RENDER_RESPONSE.
> >
> 
> Yes, it it not possible to do navigation in RENDER_RESPONSE, because
> render response phase already happen, so there exists navigation but
> there is no rendering. So, a redirect is the only way to trigger a
> navigation in this case, but this means debug information must be
> saved somewhere. All that is responsible of the ExceptionHandler used.
> 
> >
> > About infinite loop: similar problem (not hadled with spec) is in
> > broadcastEvents - listener can queue the same event again. Please see
> > code in javax.faces.component.UIViewRoot.broadcastEvents(FacesContext,
> > PhaseId). Situation in exception handler is similar: throwing a
> > exception again leads to queue new ExceptionQueuedEvent.
> > Maybe we should think about this problem (inifinite queuing) generally,
> > extract the code from UIViewRoot.broadcastEvents and solve it everywhere
> > in code base for all queue operations.
> >
> 
> I don't see any problems with the algorithm proposed related to
> infinite loops from a theorical point of view, because after all, if
> the exception handler call UIViewRoot.broadcastEvents, it is
> responsibility of the exception handler to deal with this gracefully.
> >From the point of view of the spec and the algorithm on section 12.3,
> jsf implementation should not do anything.
> 
> Anyway, I have seen some problems in our current error handling
> algorithm. Technically it comply with the spec, but note the spec
> unfortunately is very poor in this part. There is some code dealing
> with ajax exceptions, that is on ErrorPageWriter that should not be
> there, because if myfaces error handling is disabled, this code is not
> executed too, and ErrorPageWriter should be called from our
> ExceptionHandler implementation in MyFaces, instead use a try {} catch
> {} block, even if errors on ResourceHandler implementation could not
> be handled (or maybe the ExceptionHandler should do its own stuff
> here?). Additionally, our wiki page to handling errors must be
> updated, but first we need to check in deep this feature.

Hi,

I did something for MYFACES-3053 - Improve error reporting and logging.
If you have some time, please take a look at subtasks of that issue -
some of them are related to current error handling algorithm.

Regards,

Kočičák
> 
> regards,
> 
> Leonardo Uribe
> 
> > Regards,
> >
> >
> > Kočičák
> >
> > Leonardo Uribe píše v Po 27. 06. 2011 v 18:02 -0500:
> >> Hi
> >>
> >> I investigated more about this problem and from a theorical point of
> >> view we can't call handle() like proposed. Why? in JSF we have the
> >> following phases by default:
> >>
> >> RESTORE_VIEW
> >> APPLY_REQUEST_VALUES
> >> PROCESS_VALIDATIONS
> >> UPDATE_MODEL_VALUES
> >> INVOKE_APPLICATION
> >> RENDER_RESPONSE
> >>
> >> and we have these methods on FacesContext:
> >>
> >> renderResponse();
> >> getRenderResponse();
> >> responseComplete();
> >> getResponseComplete();
> >>
> >> If a ViewExpiredException is thrown on RESTORE_VIEW, and a call to
> >> handle() occur, everything that happens inside the ExceptionHandler is
> >> its responsibility. If handleNavigation() is called inside the
> >> handler, that handler should check for a view set or
> >> getResponseComplete return true and deal with it. Now, if the handler
> >> does not do anything, the following phases:
> >>
> >> APPLY_REQUEST_VALUES
> >> PROCESS_VALIDATIONS
> >> UPDATE_MODEL_VALUES
> >> INVOKE_APPLICATION
> >> RENDER_RESPONSE
> >>
> >> By definition, these phases should check always if there is a view and
> >> if it is not, just throw ViewNotFoundException, aborting the current
> >> phase. Note it is responsibility of the phase to do the check. In that
> >> way the ExceptionHandler could have the chance to handle it.
> >>
> >> I tried do something different, but it is not possible because is
> >> inconsistent with the intention of ExceptionHandler. How can an
> >> ExceptionHandler handle exceptions thrown by itself without the risk
> >> of get stuck in a loop?
> >>
> >> regards,
> >>
> >> Leonardo Uribe
> >>
> >> 2011/6/27 Leonardo Uribe <lu4242@gmail.com>:
> >> > Hi
> >> >
> >> > 2011/6/27 Martin Koci <martin.kocicak.koci@gmail.com>:
> >> >> Hi,
> >> >> Leonardo Uribe píše v Ne 26. 06. 2011 v 21:29 -0500:
> >> >>> Hi
> >> >>>
> >> >>> I have checked all issues and this is what I think about:
> >> >>>
> >> >>> - MYFACES-3189  and MYFACES-3188 : I don't see any difference between
> >> >>> both of them.
> >> >>
> >> >> Yes, those are caused by same problem, but I reported it separately
> >> >> because MYFACES-3188 suggest only improvement in "robustness" area,
> >> >> MYFACES-3189 is about navigation algorithm.
> >> >>
> >> >
> >> > Ok, now I get it. I believe one patch will fix MYFACES-3188 and
> >> > MYFACES-3189 should be closed as invalid, because we can really do
> >> > anything on the navigation algorithm.
> >> >
> >> >>> The navigation algorithm is ok. I think with just log a
> >> >>> warning message before render response phase saying the view is
null
> >> >>> is ok. I think it is reasonable to throw a custom myfaces exception
> >> >>> here because is viewRoot is null on this point, there is no way
render
> >> >>> response phase could work. Maybe a ViewNotFoundException? I don't
see
> >> >>> any incompatibility with the spec, but at least we should notify
the
> >> >>> EG about this possible problem, because the exception class should
be
> >> >>> on javax.faces.application, and the algorithm should be clear about
> >> >>> this.
> >> >>>
> >> >>
> >> >> Exception: can be this exception also handled with custom exception
> >> >> handler? For example: two views (ViewExpired.xhtml and
> >> >> NiceErrorView.xhtml) packaged in a .jar. For some reason this archive
> >> >> will no be deployed. Then if ViewExpired occurs:
> >> >>
> >> >> -- exception handler navigates to ViewExpired.xhtml ->
> >> >> ViewNotFoundException
> >> >> -- exception handler handles ViewNotFoundException and navigates to
> >> >> NiceErrorView.xhtml -> new  ViewNotFoundException is throw.
> >> >>
> >> >
> >> > Good idea!. The only thing here is we need some special logic to deal with
> >> > this. The most difficult problem is we can't keep track of the view navigations
> >> > that cause ViewNotFoundException on the same request, but we can
> >> > set a limit of how many ViewNotFoundException can be handled before
> >> > jump to the default error page writer. This looks each time more like something
> >> > to be included on the spec.
> >> >
> >> > regards,
> >> >
> >> > Leonardo
> >> >
> >> >>
> >> >>
> >> >>
> >> >>> - MYFACES-3105 : Checked and fixed.
> >> >>>
> >> >>> Fortunately, there is no errors on the algorithm, but anyway we
can
> >> >>> expect more reports about ViewExpiredException in the future, because
> >> >>> it is a common exception that occur in typical scenarios.
> >> >>>
> >> >>> regards,
> >> >>>
> >> >>> Leonardo Uribe
> >> >>>
> >> >>> 2011/6/26 Martin Koci <martin.kocicak.koci@gmail.com>:
> >> >>> > Hi,
> >> >>> >
> >> >>> > MYFACES-3189  and MYFACES-3188 are reproducible : please see
comment at
> >> >>> > MYFACES-3189. The mojara example works ok, thats not the problem.
The
> >> >>> > problem is when you makes a typo in name of view for handleNavigation
> >> >>> > method.
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>> > MYFACES-3105 seems fixed in current 2.1.2-SNAPSHOT
> >> >>> >
> >> >>> > Regards,
> >> >>> >
> >> >>> > Kočičák
> >> >>> >
> >> >>> > Leonardo Uribe píše v So 25. 06. 2011 v 11:49 -0500:
> >> >>> >> Hi
> >> >>> >>
> >> >>> >> I have tried to reproduce them without success. I know
the navigation
> >> >>> >> code and everything seems to be correct. Do you have a
test case for
> >> >>> >> this one? I tried the bundled sample from mojarra and
it works.
> >> >>> >>
> >> >>> >> regards,
> >> >>> >>
> >> >>> >> Leonardo Uribe
> >> >>> >>
> >> >>> >> 2011/6/25 Martin Koci <martin.kocicak.koci@gmail.com>
> >> >>> >>         Hi,
> >> >>> >>
> >> >>> >>         please take a look at:
> >> >>> >>
> >> >>> >>
> >> >>> >>         https://issues.apache.org/jira/browse/MYFACES-3189
> >> >>> >>         https://issues.apache.org/jira/browse/MYFACES-3188
> >> >>> >>         https://issues.apache.org/jira/browse/MYFACES-3105
> >> >>> >>
> >> >>> >>         I'm not very familiar with navigation implementation
- I
> >> >>> >>         cannot provide
> >> >>> >>         meaningful patches here.
> >> >>> >>
> >> >>> >>
> >> >>> >>         Thanks,
> >> >>> >>
> >> >>> >>
> >> >>> >>         Kočičák
> >> >>> >>
> >> >>> >>
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>>
> >> >>
> >> >>
> >> >>
> >> >
> >>
> >
> >
> >
> 



Mime
View raw message