FullAjaxExceptionHandler => OmniExceptionHandler

[pizzi80]

trafficstars

Why not handle both ajax and non-ajax exceptions inside a unique OmniExceptionHandler ?

I had to override the FullAjaxExceptionHandler and copy/paste some portions of code to properly handle the non-ajax exceptions in the same place!

Otherwise with the current OmniFaces solution (FullAjaxExceptionHandler+FacesExceptionFilter) you have the following situation:

1. FacesExceptionFilter logs the exception
2. Tomcat Error report Valve logs the exception

[BalusC]

jakarta.faces.context.ExceptionHandler API doesn't support non-Faces requests in first place. There the FacesExceptionFilter is for. And with FacesExceptionFilter in place there's no reason to repeat same logic in FullAjaxExceptionHandler. Whilst the idea is nice, the OmniExceptionHandler would still not be able to handle non-Faces requests and you would still need the FacesExceptionFilter.

I had to override the FullAjaxExceptionHandler and copy/paste some portions of code to properly handle the non-ajax exceptions in the same place!

The original intent of FacesExceptionFilter is that this should not have been necessary. Can you elaborate how exactly the FacesExceptionFilter is shortcoming here?

[pizzi80]

Probably, it's time to improve Faces exception handling to avoid the need of FacesExceptionFilter

The original intent of FacesExceptionFilter is that this should not have been necessary. Can you elaborate how exactly the FacesExceptionFilter is shortcoming here?

I don't want that an exception hits Tomcat's (or whatever container) exception handling because I have a dynamic error page location (and other things to do) based on the location of the jsf view file

Obviously the algorithm is the same both for ajax and normal exception (at the end of the day an exception is an exception... ajax or on ajax :) )

In general, for a developer it's better to handle things of the same "nature" in a unique place.

At the moment you have to fight againts:

1. Faces warning log(s) ... now fixed with your last PR :top:
2. Faces ExceptionHandler
3. FacesExceptionFilter
4. FullAjaxExceptionHalder
5. TomcatErrorReportValve
6. web.xml configuration

[BalusC]

based on the location of the jsf view file

Hmm. Fair. But then you still need to deal with non-Faces requests. There's no way that you can use a ExceptionHandler for this. So in the end you still need a minimum of two exception handlers, a Faces ExceptionHandler and a Servlet Filter. Do note that the FullAjaxExceptionHandler is a Faces ExceptionHandler already. And, the TomcatErrorReportValve (or whatever server dependent) shouldn't be necessary if web.xml error page config has at least a default error page.

[pizzi80]

But then you still need to deal with non-Faces requests

Servlet and Rest ?

ok so for that it's ok to have a catch all inside web.xml that points to the default error.xhtml page

So in the end you still need a minimum of two exception handlers, a Faces ExceptionHandler and a Servlet Filter

Why do we still need a Servlet Filter?

Can't we change the core Faces ExceptionHandler to do at least the same work of OmniFaces FacesExceptionFilter ?

In this way whatever happen inside JSF can be managed with JSF things.

If one develop a JSF only webapp without any other Servlet has a single place to handle everything and a single line of code in web.xml just in case ..

And, the TomcatErrorReportValve (or whatever server dependent) shouldn't be necessary if web.xml error page config has at least a default error page.

Interesting ... didn't know ....

I should try to remove it and make some tests ;)

[BalusC]

Why do we still need a Servlet Filter?

For non-Faces requests.

Can't we change the core Faces ExceptionHandler to do at least the same work of OmniFaces FacesExceptionFilter ?

No, this has the same problem as FullAjaxExceptionHandler/OmniExceptionHandler: it still wouldn't be able to catch exceptions on non-Faces requests.

"non-Faces requests" are those requests which didn't hit the FacesServlet. Such as plain/default Servlet and JAX-RS indeed, as well as the majority of 4xx errors. The Faces ExceptionHandler is only managed/triggered by the FacesServlet, not by the servlet container.

[pizzi80]

m m m

Due to the fact that at "low-level" everything is a servlet

it would be better to create a Servlet that act as a global "ServletExceptionHandler" ?

and declare it in web.xml

<error-page> 
  <exception-type>java.lang.Exception</exception-type> 
  <location>/error</location> 
</error-page>

@WebServlet(urlPatterns = "/error")
public class ServletExceptionHandler extends HttpServlet { ... }

-> Servlet -> Exception -> ServletExceptionHandler -> Rest -> Exception -> ServletExceptionHandler -> Faces -> Exception -> ServletExceptionHandler -> Faces Ajax -> Exception -> ServletExceptionHandler

[BalusC]

It won't have access to FacesContext nor be able to handle Faces Ajax exceptions. These by default don't throw any exception that a filter or error page servlet can catch.

[pizzi80]

For ajax we could rethrow it inside the Default JSF ExceptionHandler

We could use CDI to inject FacesContext ... ?

[BalusC]

For ajax we could rethrow it inside the Default JSF ExceptionHandler

You can throw it but an error page servlet/filter running outside FacesContext won't be able to render a proper Faces Ajax response (without reinventing/copying a lot of boileplate code already done by Faces). You basically need to send a redirect which is a bad idea.

We could use CDI to inject FacesContext ... ?

FacesContext is created by FacesServlet and therefore not available in other servlets/filters.

[BalusC]

Summarized: it should be sufficient to have:

• web.xml for error page config per exception type (and a default error page!)
• FullAjaxExceptionHandler to deal with exceptions thrown during Faces Ajax requests
• FacesExceptionFilter to deal with exceptions thrown during non-Faces requests and Faces non-Ajax requests

In your specific case you also seem to want to be able to locate a specific error page depending on view ID (servlet path). This is indeed not supported by web.xml config, so best what we could do is to introduce a new context param in web.xml which both the FullAjaxExceptionHandler and FacesExceptionFilter can utilize. Do note that there are already two existing context params which are shared by them (exception types to unwrap and exception types to ignore in logging).

[pizzi80]

Thanks for the discussion!

This is exactly my final configuration, but I have an ExceptionHandler that extends the FullAjaxExceptionHandler to fit my needs.

I don't know if with a custom context param we could cover all the possible situations... :roll_eyes:

I also have defined a custom exception for when i want to send a 500 error code during an ajax exception to let javascript handle the situation (PrimeFaces onerror at the moment)

[melloware]

@pizzi80 @BalusC we got this PrimeFaces request and I don't quite understand what the user is trying to do with error pages and I am also not a Tomcat user can one of you make sense of this? https://github.com/primefaces/primefaces/issues/11641

[BalusC]

Responded over there :+1:

[BalusC]

Summarized:

1. Configuration is indeed a pain point when you want to cover any exception possible (non-Faces, Faces Ajax, Faces non-Ajax). Ideally it should be able to automatically register/activate FacesExceptionFilter when FullAjaxExceptionFilter is registered in faces-config.xml. I'll work on that for a next OmniFaces version.
2. There was apparently a custom impl to pick a specific error page depending on view ID. After a second thought I disagree this approach. I recommend to instead throw a sufficiently specific exception so that the desired error page config in web.xml is automatically matched.

[pizzi80]

1. Configuration is indeed a pain point when you want to cover any exception possible (non-Faces, Faces Ajax, Faces non-Ajax). Ideally it should be able to automatically register/activate FacesExceptionFilter when FullAjaxExceptionFilter is registered in faces-config.xml. I'll work on that for a next OmniFaces version.

:muscle:

2. There was apparently a custom impl to pick a specific error page depending on view ID. After a second thought I disagree this approach. I recommend to instead throw a sufficiently specific exception so that the desired error page config in web.xml is automatically matched.

ok, I have to check if with a hierarchy of exceptions it is possible to handle all possible situations