New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow FullAjaxExceptionHandler to be configured to unwrap more Exception types #196

Closed
kalgon opened this Issue Jan 15, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@kalgon

kalgon commented Jan 15, 2016

One of the reasons I extend FullAjaxExceptionHandler is to add more Exceptiontypes to unwrap like this:

import static org.omnifaces.util.Exceptions.unwrap;

public class ExtendedFullAjaxExceptionHandler extends FullAjaxExceptionHandler {
    ...
    @Override
    protected Throwable findExceptionRootCause(FacesContext context, Throwable exception) {
        return unwrap(unwrap(super.findExceptionRootCause(context, exception), EJBException.class), RollbackException.class);
    }   
}

After that, you also need to write the corresponding ExceptionHandlerFactory.

Do you think it would be a good idea if FullAjaxExceptionHandler could use a list of Exception types to unwrap coming from web.xml or faces-config.xml ? We do that already for error-pages.

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Jan 15, 2016

Member

EJBException? You might want to read this for a better solution: Handling service layer exception in JSF

Member

BalusC commented Jan 15, 2016

EJBException? You might want to read this for a better solution: Handling service layer exception in JSF

@kalgon

This comment has been minimized.

Show comment
Hide comment
@kalgon

kalgon Jan 15, 2016

I agree for EJBException, you can annotate exceptions with @ApplicationException or declare them in ejb-jar.xml if you don't have access to the source code (I tend to re-use exceptions like NoSuchElementException for 404). Lazy as I am, I just unwrap EJBException.

The reason I unwrap RollbackException is to catch exceptions like OptimisticLockException to explain to the user what happened (concurrent modification) and invite him to reload his page and start over what he was doing.

kalgon commented Jan 15, 2016

I agree for EJBException, you can annotate exceptions with @ApplicationException or declare them in ejb-jar.xml if you don't have access to the source code (I tend to re-use exceptions like NoSuchElementException for 404). Lazy as I am, I just unwrap EJBException.

The reason I unwrap RollbackException is to catch exceptions like OptimisticLockException to explain to the user what happened (concurrent modification) and invite him to reload his page and start over what he was doing.

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Mar 5, 2016

Member

It's available in today's 2.3 snapshot. Let me know if it does the job you asked for.

The syntax is

<context-param>
    <param-name>org.omnifaces.EXCEPTION_TYPES_TO_UNWRAP</param-name>
    <param-value>javax.ejb.EJBException,javax.persistence.RollbackException</param-value>
</context-param>

Those will be added on top of FacesException and ELException (and thus won't override them). For convenience, the FacesExceptionFilter (for non-ajax requests) will also use them.

Member

BalusC commented Mar 5, 2016

It's available in today's 2.3 snapshot. Let me know if it does the job you asked for.

The syntax is

<context-param>
    <param-name>org.omnifaces.EXCEPTION_TYPES_TO_UNWRAP</param-name>
    <param-value>javax.ejb.EJBException,javax.persistence.RollbackException</param-value>
</context-param>

Those will be added on top of FacesException and ELException (and thus won't override them). For convenience, the FacesExceptionFilter (for non-ajax requests) will also use them.

@kalgon

This comment has been minimized.

Show comment
Hide comment
@kalgon

kalgon Mar 8, 2016

I tested it and it's working as I expected. Thanks!

kalgon commented Mar 8, 2016

I tested it and it's working as I expected. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment