Skip to content
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

Custom ExceptionMapper not invoked when Hibernate rollback #949

Closed
paukiatwee opened this issue Mar 21, 2015 · 4 comments · Fixed by #958
Closed

Custom ExceptionMapper not invoked when Hibernate rollback #949

paukiatwee opened this issue Mar 21, 2015 · 4 comments · Fixed by #958
Milestone

Comments

@paukiatwee
Copy link

Using DW 0.8.0, Java 8

I have one custom exception mapper to handle hibernate exception:

import org.hibernate.exception.ConstraintViolationException;


@Provider
public class SQLConstraintViolationExceptionMapper implements ExceptionMapper<ConstraintViolationException> {
    ....
}

I registered the mapper https://github.com/paukiatwee/budgetapp/blob/master/src/main/java/io/budgetapp/BudgetApplication.java#L143

When org.hibernate.exception.ConstraintViolationException exception is thrown when foreign key violation occurred, SQLConstraintViolationExceptionMapper is no called.

You can reproduce the problem by running this integration test https://github.com/paukiatwee/budgetapp/blob/master/src/test/java/io/budgetapp/resource/CategoryResourceIT.java#L65

This might caused by https://github.com/dropwizard/dropwizard/blob/master/dropwizard-hibernate/src/main/java/io/dropwizard/hibernate/UnitOfWorkApplicationListener.java#L87 exception not handled by container.

Working fine for DW 0.7.0

@paukiatwee paukiatwee changed the title Custom ExceptionMapper not invoked Custom ExceptionMapper not invoked when Hibernate rollback Mar 21, 2015
@arteam
Copy link
Member

arteam commented Mar 21, 2015

I can confirm that issue.

Unfortunately, I can't give a solution now. The problem lies in the update to Jersey 2 in 0.8.0.
Seems to be UnitOfWorkApplicationListener doesn't work with exception mappers.

@arteam
Copy link
Member

arteam commented Mar 27, 2015

I've made another look and can give a workaround for you.

In 0.8.0 in UnitOfWorkApplicationListener we commit a Hibernate transaction after a request has been processed and response processing has been started. We don't have an event like BEFORE_RESOURCE_METHOD_FINISHED, so we are forced to make a commit (with possible errors) during response processing.

In the Jersey architecture at this stage exception mappers are not taken into account, but mappers that implement the interface ResponseErrorMapper are.
They are also not processed by default, so you need to enable this feature in configuration:

environment.jersey().property(ServerProperties.PROCESSING_RESPONSE_ERRORS_ENABLED, true);

After that you can refactor your exception mapper something like that:

@Provider
public class SQLConstraintViolationExceptionMapper implements ResponseErrorMapper {

    private static final Logger LOGGER = LoggerFactory.getLogger(SQLConstraintViolationExceptionMapper.class);

    @Override
    public Response toResponse(Throwable e) {
        return Response.status(Response.Status.BAD_REQUEST)
                .entity(Collections.singletonMap("errors",
                        Collections.singletonMap("message", getMessage((ConstraintViolationException) e))))
                .build();
    }

    private List<String> getMessage(ConstraintViolationException e) {

        LOGGER.error(e.getMessage(), e);

        // a hack to convert exception to friendly error message
        if ("fk_budgets_categories".equalsIgnoreCase(e.getConstraintName())) {
            return Collections.singletonList("Failed to delete category due to references to existing budget(s).");
        } else if ("fk_transactions_budgets".equalsIgnoreCase(e.getConstraintName())) {
            return Collections.singletonList("Failed to delete budget due to references to existing transaction(s).");
        }
        return Collections.singletonList(e.getMessage());
    }
}

Could you test this approach? If it works for you, I will add information about response error mappers
into the 0.8.0 migration guide.

@paukiatwee
Copy link
Author

The issue is fixed after update based on your comment commit.

However, I think the proper way to fix this is to rethrow exception as org.glassfish.jersey.server.internal.process.MappableException as the doc mention:

 * A runtime exception that contains a cause, a checked or runtime exception,
 * that may be mapped to a {@link javax.ws.rs.core.Response} instance.
 * <p>
 * The runtime will catch such exceptions and attempt to map the cause
 * exception to a registered {@link javax.ws.rs.ext.ExceptionMapper} that
 * provides an appropriate {@link javax.ws.rs.core.Response} instance.

@arteam
Copy link
Member

arteam commented Mar 28, 2015

That actually seems like a better solution. Jersey will rethrow the original exception if it's unmapped and map to Response if it's mapped. Thanks for pointers.

I will try to create a PR for fixing that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants