From d96f118c6e6614c7db5ef2494301ba088ce8fab1 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Wed, 5 Apr 2023 12:17:22 +0300 Subject: [PATCH 1/2] Sanitize the behavior of provided ExceptionMapper classes in dev-mode User provided ExceptionMapper classes now takes precedence over the Quarkus provided NotFoundExceptionMapper. This is done in a way that does not change anything but this specific behavior, everything else concerning exception handling continues to work as before. Note also that this does not change the spec compliance in any way Fixes: #7883 --- .../QuarkusDefaultExceptionHandlingTest.java | 64 +++++++++++++++++ .../UserProvidedExceptionHandlingTest.java | 72 +++++++++++++++++++ .../runtime/NotFoundExceptionMapper.java | 9 ++- .../server/core/RuntimeExceptionMapper.java | 22 +++++- 4 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java new file mode 100644 index 0000000000000..4c79ee106f685 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java @@ -0,0 +1,64 @@ +package io.quarkus.resteasy.reactive.server.test.devmode; + +import static org.hamcrest.CoreMatchers.containsString; + +import java.util.function.Supplier; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class QuarkusDefaultExceptionHandlingTest { + + @RegisterExtension + static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(Resource.class); + } + + }); + + @Test + public void testDefaultErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception") + .then() + .statusCode(500) + .body(containsString("Internal Server Error"), containsString("dummy exception")); + } + + @Test + public void testNotFoundErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception2") + .then() + .statusCode(404) + .body(containsString("404 - Resource Not Found")); + } + + @Path("test") + @Produces(MediaType.TEXT_PLAIN) + @Consumes(MediaType.TEXT_PLAIN) + public static class Resource { + + @Path("exception") + @GET + @Produces("text/html") + public String exception() { + throw new RuntimeException("dummy exception"); + } + } +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java new file mode 100644 index 0000000000000..1d241f9287284 --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java @@ -0,0 +1,72 @@ +package io.quarkus.resteasy.reactive.server.test.devmode; + +import java.util.function.Supplier; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class UserProvidedExceptionHandlingTest { + + @RegisterExtension + static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(Resource.class, CustomExceptionMapper.class); + } + + }); + + @Test + public void testDefaultErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception") + .then() + .statusCode(999); + } + + @Test + public void testNotFoundErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception2") + .then() + .statusCode(999); + } + + @Path("test") + @Produces(MediaType.TEXT_PLAIN) + @Consumes(MediaType.TEXT_PLAIN) + public static class Resource { + + @Path("exception") + @GET + @Produces("text/html") + public String exception() { + throw new RuntimeException("dummy exception"); + } + } + + @Provider + public static class CustomExceptionMapper implements ExceptionMapper { + @Override + public Response toResponse(Exception exception) { + return Response.status(999).build(); + } + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java index 80a154f2bbce1..44cf5563886f7 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java @@ -34,6 +34,7 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.common.util.ServerMediaType; import org.jboss.resteasy.reactive.server.ServerExceptionMapper; +import org.jboss.resteasy.reactive.server.core.RuntimeExceptionMapper; import org.jboss.resteasy.reactive.server.core.request.ServerDrivenNegotiation; import org.jboss.resteasy.reactive.server.handlers.RestInitialHandler; import org.jboss.resteasy.reactive.server.mapping.RequestMapper; @@ -81,8 +82,12 @@ public MethodDescription(String method, String fullPath, String produces, String } } - @ServerExceptionMapper(value = NotFoundException.class, priority = Priorities.USER + 1) - public Response toResponse(HttpHeaders headers) { + // we don't use NotFoundExceptionMapper here because that would result in users not being able to provide their own catch-all exception mapper in dev-mode, see https://github.com/quarkusio/quarkus/issues/7883 + @ServerExceptionMapper(priority = Priorities.USER + 1) + public Response toResponse(Throwable t, HttpHeaders headers) { + if (!(t instanceof NotFoundException)) { + return RuntimeExceptionMapper.IGNORE_RESPONSE; + } if ((classMappers == null) || classMappers.isEmpty()) { return respond(headers); } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java index 04be9002bf53b..cebd924463489 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java @@ -20,6 +20,7 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.ResteasyReactiveClientProblem; import org.jboss.resteasy.reactive.common.model.ResourceExceptionMapper; +import org.jboss.resteasy.reactive.server.jaxrs.ResponseBuilderImpl; import org.jboss.resteasy.reactive.server.mapping.RuntimeResource; import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveAsyncExceptionMapper; import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveExceptionMapper; @@ -42,6 +43,9 @@ public class RuntimeExceptionMapper { private final List> nonBlockingProblemPredicate; private final Set> unwrappedExceptions; + public static final Response IGNORE_RESPONSE = new ResponseBuilderImpl().status(666).header( + "RR_EX_IGN", "true").build(); + public RuntimeExceptionMapper(ExceptionMapping mapping, ClassLoader classLoader) { try { mappers = new HashMap<>(); @@ -94,9 +98,21 @@ public void mapException(Throwable throwable, ResteasyReactiveRequestContext con } else { response = exceptionMapper.toResponse(mappedException); } - context.setResult(response); - logBlockingErrorIfRequired(mappedException, context); - logNonBlockingErrorIfRequired(mappedException, context); + // this special case is used in order to ignore the mapping of built-in mappers and let the exception handling proceed to higher levels + if ((IGNORE_RESPONSE == response)) { + if (isWebApplicationException) { + context.setResult(((WebApplicationException) throwable).getResponse()); + log.debug("Application failed the request", throwable); + } else { + logBlockingErrorIfRequired(throwable, context); + logNonBlockingErrorIfRequired(throwable, context); + context.handleUnmappedException(throwable); + } + } else { + context.setResult(response); + logBlockingErrorIfRequired(mappedException, context); + logNonBlockingErrorIfRequired(mappedException, context); + } return; } if (isWebApplicationException) { From 96821995bc5ac351b7b8b3fad5e0349ee7026b8d Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Wed, 5 Apr 2023 11:29:59 +0300 Subject: [PATCH 2/2] Fix javadoc of ServerExceptionMapper --- .../reactive/server/ServerExceptionMapper.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/ServerExceptionMapper.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/ServerExceptionMapper.java index 00744a8ba2dd1..af60f08cb51bd 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/ServerExceptionMapper.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/ServerExceptionMapper.java @@ -15,17 +15,16 @@ /** * When used on a method, then an implementation of {@link javax.ws.rs.ext.ExceptionMapper} is generated * that calls the annotated method with the proper arguments. - * + *

* When the annotation is placed on a method that is not a JAX-RS Resource class, the method handles exceptions in global - * fashion - * (as do regular JAX-RS {@code ExceptionMapper} implementations). + * fashion (as do regular JAX-RS {@code ExceptionMapper} implementations). * However, when it is placed on a method of a JAX-RS Resource class, the method is only used to handle exceptions originating * from * that JAX-RS Resource class. * Methods in a JAX-RS class annotated with this annotation will be used first when determining how to handle a thrown * exception. * This means that these methods take precedence over the global {@link javax.ws.rs.ext.ExceptionMapper} classes. - * + *

* In addition to the exception being handled, an annotated method can also declare any of the following * parameters (in any order): *

    @@ -39,10 +38,11 @@ * * When {@code value} is not set, then the handled Exception type is deduced by the Exception type used in the method parameters * (there must be exactly one Exception type in this case). - * + *

    * The return type of the method must be either be of type {@code Response}, {@code Uni}, {@code RestResponse} or * {@code Uni}. */ + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface ServerExceptionMapper {