Leonardo Uribe píše v Út 28. 06. 2011 v 16:11 -0500: > Hi > > 2011/6/28 Martin Koci <[email protected]>: > > 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 <[email protected]>: > >> > Hi > >> > > >> > 2011/6/27 Martin Koci <[email protected]>: > >> >> 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 <[email protected]>: > >> >>> > 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 <[email protected]> > >> >>> >> 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 > >> >>> >> > >> >>> >> > >> >>> > > >> >>> > > >> >>> > > >> >>> > >> >> > >> >> > >> >> > >> > > >> > > > > > > >
