Skip to content

Commit

Permalink
Sanitize the behavior of provided ExceptionMapper classes in dev-mode
Browse files Browse the repository at this point in the history
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: quarkusio#7883
  • Loading branch information
geoand committed Apr 5, 2023
1 parent 9685b22 commit e631d34
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package io.quarkus.resteasy.reactive.server.test.devmode;

import static org.hamcrest.CoreMatchers.containsString;

import java.util.function.Supplier;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.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");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package io.quarkus.resteasy.reactive.server.test.devmode;

import java.util.function.Supplier;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.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<Exception> {
@Override
public Response toResponse(Exception exception) {
return Response.status(999).build();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,6 +43,9 @@ public class RuntimeExceptionMapper {
private final List<Predicate<Throwable>> nonBlockingProblemPredicate;
private final Set<Class<? extends Throwable>> 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<>();
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit e631d34

Please sign in to comment.