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

Introduce @UnwrapException for Quarkus REST #42094

Merged
merged 1 commit into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions docs/src/main/asciidoc/rest.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,40 @@
Users however can opt into interceptors by adding the corresponding annotations to the method.
====

[TIP]
====
When mapping an exception to a `@ServerExceptionMapper` method, the cause of the exception normally does not come into play.

However, some exception types in Java only serve as wrappers for other exceptions. Often, checked exceptions are wrapped into `RuntimeException` just to not have them declared in method `throws` parameters.
Working with `CompletionStage` for example, will require `CompletionException`. There are many such exception types that are just wrappers around the real cause of the exception.

If you wish to make sure your exception mapper is called for your exception type even when it is wrapped by one of those wrapper exceptions, you can use `@UnwrapException` on the exception wrapper type:

Check warning on line 2281 in docs/src/main/asciidoc/rest.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer. Raw Output: {"message": "[Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer.", "location": {"path": "docs/src/main/asciidoc/rest.adoc", "range": {"start": {"line": 2281, "column": 1}}}, "severity": "INFO"}

Check warning on line 2281 in docs/src/main/asciidoc/rest.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsWarnings] Consider using 'want' rather than 'wish' unless updating existing content that uses the term. Raw Output: {"message": "[Quarkus.TermsWarnings] Consider using 'want' rather than 'wish' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/rest.adoc", "range": {"start": {"line": 2281, "column": 8}}}, "severity": "WARNING"}

Check warning on line 2281 in docs/src/main/asciidoc/rest.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsWarnings] Consider using 'verify' rather than 'make sure' unless updating existing content that uses the term. Raw Output: {"message": "[Quarkus.TermsWarnings] Consider using 'verify' rather than 'make sure' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/rest.adoc", "range": {"start": {"line": 2281, "column": 16}}}, "severity": "WARNING"}

[source,java]
----
public class MyExceptionWrapper extends RuntimeException {
public MyExceptionWrapper(Exception cause) {
super(cause);
}
}
----

If you don't control that exception wrapper type, you can place the annotation on any class and specify the exception wrapper types it applies to as annotation parameter:

Check warning on line 2292 in docs/src/main/asciidoc/rest.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/rest.adoc", "range": {"start": {"line": 2292, "column": 147}}}, "severity": "INFO"}

[source,java]
----
@UnwrapException({CompletionException.class, RuntimeException.class})
public class Mapper {

@ServerExceptionMapper
public Response handleMyException(MyException x) {
// ...
}

}
----
====

[NOTE]
====
Εxception mappers defined in REST endpoint classes will only be called if the exception is thrown in the same class. If you want to define global exception mappers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.jboss.jandex.IndexView;
import org.jboss.jandex.Indexer;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;
import org.jboss.resteasy.reactive.common.core.BlockingNotAllowedException;
import org.jboss.resteasy.reactive.common.model.ResourceContextResolver;
import org.jboss.resteasy.reactive.common.model.ResourceExceptionMapper;
Expand All @@ -33,6 +34,7 @@
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;
import org.jboss.resteasy.reactive.common.processor.scanning.ApplicationScanningResult;
import org.jboss.resteasy.reactive.common.processor.scanning.ResteasyReactiveInterceptorScanner;
import org.jboss.resteasy.reactive.server.UnwrapException;
import org.jboss.resteasy.reactive.server.core.ExceptionMapping;
import org.jboss.resteasy.reactive.server.model.ContextResolvers;
import org.jboss.resteasy.reactive.server.model.ParamConverterProviders;
Expand Down Expand Up @@ -82,6 +84,9 @@
*/
public class ResteasyReactiveScanningProcessor {

private static final DotName EXCEPTION = DotName.createSimple(Exception.class);
private static final DotName RUNTIME_EXCEPTION = DotName.createSimple(RuntimeException.class);

public static final Set<DotName> CONDITIONAL_BEAN_ANNOTATIONS;

static {
Expand Down Expand Up @@ -118,11 +123,55 @@ public void accept(ResourceInterceptors interceptors) {
}

@BuildStep
public List<UnwrappedExceptionBuildItem> defaultUnwrappedException() {
public List<UnwrappedExceptionBuildItem> defaultUnwrappedExceptions() {
return List.of(new UnwrappedExceptionBuildItem(ArcUndeclaredThrowableException.class),
new UnwrappedExceptionBuildItem(RollbackException.class));
}

@BuildStep
public void applicationSpecificUnwrappedExceptions(CombinedIndexBuildItem combinedIndexBuildItem,
BuildProducer<UnwrappedExceptionBuildItem> producer) {
IndexView index = combinedIndexBuildItem.getIndex();
for (AnnotationInstance instance : index.getAnnotations(UnwrapException.class)) {
AnnotationValue value = instance.value();
if (value == null) {
// in this case we need to use the class where the annotation was placed as the exception to be unwrapped

AnnotationTarget target = instance.target();
if (target.kind() != AnnotationTarget.Kind.CLASS) {
throw new IllegalStateException(
"@UnwrapException is only supported on classes. Offending target is: " + target);
}
ClassInfo classInfo = target.asClass();
ClassInfo toCheck = classInfo;
boolean isException = false;
while (true) {
DotName superDotName = toCheck.superName();
if (EXCEPTION.equals(superDotName) || RUNTIME_EXCEPTION.equals(superDotName)) {
isException = true;
break;
}
toCheck = index.getClassByName(superDotName);
if (toCheck == null) {
break;
}
}
if (!isException) {
throw new IllegalArgumentException(
"Using @UnwrapException without a value is only supported on exception classes. Offending target is '"
+ classInfo.name() + "'.");
}

producer.produce(new UnwrappedExceptionBuildItem(classInfo.name().toString()));
} else {
Type[] exceptionTypes = value.asClassArray();
for (Type exceptionType : exceptionTypes) {
producer.produce(new UnwrappedExceptionBuildItem(exceptionType.name().toString()));
}
}
}
}

@BuildStep
public ExceptionMappersBuildItem scanForExceptionMappers(CombinedIndexBuildItem combinedIndexBuildItem,
ApplicationResultBuildItem applicationResultBuildItem,
Expand All @@ -137,7 +186,7 @@ public ExceptionMappersBuildItem scanForExceptionMappers(CombinedIndexBuildItem
exceptions.addBlockingProblem(BlockingOperationNotAllowedException.class);
exceptions.addBlockingProblem(BlockingNotAllowedException.class);
for (UnwrappedExceptionBuildItem bi : unwrappedExceptions) {
exceptions.addUnwrappedException(bi.getThrowableClass().getName());
exceptions.addUnwrappedException(bi.getThrowableClassName());
}
if (capabilities.isPresent(Capability.HIBERNATE_REACTIVE)) {
exceptions.addNonBlockingProblem(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
package io.quarkus.resteasy.reactive.server.test.customexceptions;

import static io.quarkus.resteasy.reactive.server.test.ExceptionUtil.removeStackTrace;
import static io.restassured.RestAssured.when;

import java.util.function.Supplier;

import jakarta.ws.rs.Path;
import jakarta.ws.rs.core.Response;

import org.jboss.resteasy.reactive.server.ServerExceptionMapper;
import org.jboss.resteasy.reactive.server.UnwrapException;
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.resteasy.reactive.server.test.ExceptionUtil;
import io.quarkus.test.QuarkusUnitTest;

public class UnwrapExceptionTest {

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(FirstException.class, SecondException.class, ThirdException.class,
FourthException.class, FifthException.class, SixthException.class,
Mappers.class, Resource.class, ExceptionUtil.class);
}
});

@Test
public void testWrapperWithUnmappedException() {
when().get("/hello/iaeInSecond")
.then().statusCode(500);
}

@Test
public void testMappedExceptionWithoutUnwrappedWrapper() {
when().get("/hello/iseInFirst")
.then().statusCode(500);

when().get("/hello/iseInThird")
.then().statusCode(500);

when().get("/hello/iseInSixth")
.then().statusCode(500);
}

@Test
public void testWrapperWithMappedException() {
when().get("/hello/iseInSecond")
.then().statusCode(900);

when().get("/hello/iseInFourth")
.then().statusCode(900);

when().get("/hello/iseInFifth")
.then().statusCode(900);
}

@Path("hello")
public static class Resource {

@Path("iseInFirst")
public String throwsISEAsCauseOfFirstException() {
throw removeStackTrace(new FirstException(removeStackTrace(new IllegalStateException("dummy"))));
}

@Path("iseInSecond")
public String throwsISEAsCauseOfSecondException() {
throw removeStackTrace(new SecondException(removeStackTrace(new IllegalStateException("dummy"))));
}

@Path("iaeInSecond")
public String throwsIAEAsCauseOfSecondException() {
throw removeStackTrace(new SecondException(removeStackTrace(new IllegalArgumentException("dummy"))));
}

@Path("iseInThird")
public String throwsISEAsCauseOfThirdException() throws ThirdException {
throw removeStackTrace(new ThirdException(removeStackTrace(new IllegalStateException("dummy"))));
}

@Path("iseInFourth")
public String throwsISEAsCauseOfFourthException() throws FourthException {
throw removeStackTrace(new FourthException(removeStackTrace(new IllegalStateException("dummy"))));
}

@Path("iseInFifth")
public String throwsISEAsCauseOfFifthException() {
throw removeStackTrace(new FifthException(removeStackTrace(new IllegalStateException("dummy"))));
}

@Path("iseInSixth")
public String throwsISEAsCauseOfSixthException() {
throw removeStackTrace(new SixthException(removeStackTrace(new IllegalStateException("dummy"))));
}
}

@UnwrapException({ FourthException.class, FifthException.class })
public static class Mappers {

@ServerExceptionMapper
public Response handleIllegalStateException(IllegalStateException e) {
return Response.status(900).build();
}
}

public static class FirstException extends RuntimeException {

public FirstException(Throwable cause) {
super(cause);
}
}

@UnwrapException
public static class SecondException extends FirstException {

public SecondException(Throwable cause) {
super(cause);
}
}

public static class ThirdException extends Exception {

public ThirdException(Throwable cause) {
super(cause);
}
}

public static class FourthException extends SecondException {

public FourthException(Throwable cause) {
super(cause);
}
}

public static class FifthException extends RuntimeException {

public FifthException(Throwable cause) {
super(cause);
}
}

public static class SixthException extends RuntimeException {

public SixthException(Throwable cause) {
super(cause);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,32 @@
import io.quarkus.builder.item.MultiBuildItem;

/**
* When an Exception of this type is thrown and no {@code jakarta.ws.rs.ext.ExceptionMapper} exists,
* When an {@link Exception} of this type is thrown and no {@code jakarta.ws.rs.ext.ExceptionMapper} exists,
* then RESTEasy Reactive will attempt to locate an {@code ExceptionMapper} for the cause of the Exception.
*/
public final class UnwrappedExceptionBuildItem extends MultiBuildItem {

private final Class<? extends Throwable> throwableClass;
private final String throwableClassName;

public UnwrappedExceptionBuildItem(Class<? extends Throwable> throwableClass) {
this.throwableClass = throwableClass;
public UnwrappedExceptionBuildItem(String throwableClassName) {
this.throwableClassName = throwableClassName;
}

public UnwrappedExceptionBuildItem(Class<? extends Throwable> throwableClassName) {
this.throwableClassName = throwableClassName.getName();
}

@Deprecated(forRemoval = true)
public Class<? extends Throwable> getThrowableClass() {
return throwableClass;
try {
return (Class<? extends Throwable>) Class.forName(throwableClassName, false,
Thread.currentThread().getContextClassLoader());
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}
}

public String getThrowableClassName() {
return throwableClassName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
* <p>
* The return type of the method must be either be of type {@code Response}, {@code Uni<Response>}, {@code RestResponse} or
* {@code Uni<RestResponse>}.
* <p>
* See also {@link UnwrapException}
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.jboss.resteasy.reactive.server;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Used to configure that an exception (or exceptions) should be unwrapped during exception handling.
* <p>
* Unwrapping means that when an {@link Exception} of the configured type is thrown and no
* {@code jakarta.ws.rs.ext.ExceptionMapper} exists,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit subtle. This unwrapping only happens when there's no registered mapper for the direct exception type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this should be mentioned in the docs? (The guide, not just the javadoc)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is implied, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find it implied, no. How I read it was that the minute we declare that an exception should be unwrapped, it will never match an exceptoin mapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to push an update with what you have in mind, I'm perfectly fine with that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to phrase this in the docs, especially given that it's already a TIP followed by a NOTE, I feel like this succession of tips/notes should be turned into sub-sections :-/

Also, I'm now realising that it should probably be a warning (or error?) if we have both an exception mapper for T and an unwrap exception for T, no? In this case, the exception will never be unwrapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm now realising that it should probably be a warning (or error?) if we have both an exception mapper for T and an unwrap exception for T, no? In this case, the exception will never be unwrapped.

Yeah, that can be a good follow-up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to phrase this in the docs, especially given that it's already a TIP followed by a NOTE, I feel like this succession of tips/notes should be turned into sub-sections :-/

Yeah, very likely so.

* then RESTEasy Reactive will attempt to locate an {@code ExceptionMapper} for the cause of the Exception.
*/
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public @interface UnwrapException {

/**
* If this is not set, the value is assumed to be the exception class where the annotation is placed
*/
Class<? extends Exception>[] value() default {};
}
Loading