Merge pull request #42290 from geoand/#42275

Don't convert WebApplicationException in REST Client when produced by a custom mapper

Author: geoand

extensions/resteasy-reactive/rest-client-jackson/deployment/src/test/java/io/quarkus/rest/client/reactive/jackson/test/BadRequestNotPropagatedTestCase.java

@@ -3,7 +3,6 @@
 import java.net.URL;
 import java.util.List;
 
-import jakarta.inject.Inject;
 import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.Path;
@@ -22,6 +21,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
+import io.quarkus.rest.client.reactive.ClientExceptionMapper;
 import io.quarkus.test.QuarkusUnitTest;
 import io.quarkus.test.common.http.TestHTTPResource;
 
@@ -55,6 +55,12 @@ public void testBadRequest() {
         Assertions.assertEquals(500, data.getStatus());
     }
 
+    @Test
+    public void testBadRequestWithCusterMapper() {
+        Response data = client.target(url.toExternalForm() + "/bad-server/with-custom-mapper").request().get();
+        Assertions.assertEquals(999, data.getStatus());
+    }
+
     @Path("/bad")
     public static class Bad {
 
@@ -75,17 +81,32 @@ public interface BadClient {
         JsonObject get(String json);
     }
 
+    @Path("/bad")
+    @RegisterRestClient(baseUri = "http://localhost:8081")
+    public interface BadClientWithCustomMapper extends BadClient {
+
+        @ClientExceptionMapper
+        static RuntimeException toException(Response response) {
+            if (response.getStatus() == 400) {
+                return new WebApplicationException(999);
+            }
+            return null;
+        }
+    }
+
     static class JsonObject {
         String name;
     }
 
     @Path("/bad-server")
     public static class BadServer {
 
-        @Inject
         @RestClient
         BadClient badClient;
 
+        @RestClient
+        BadClientWithCustomMapper badClientWithCustomMapper;
+
         @GET
         public JsonObject get() {
             try {
@@ -104,5 +125,12 @@ public JsonObject get() {
                 return null;
             }
         }
+
+        @GET
+        @Path("with-custom-mapper")
+        public JsonObject getWithCustomMapper() {
+            return badClientWithCustomMapper.get("{name:foo}");
+        }
+
     }
 }

extensions/resteasy-reactive/rest-client/runtime/src/main/java/io/quarkus/rest/client/reactive/runtime/MicroProfileRestClientResponseFilter.java

@@ -1,5 +1,7 @@
 package io.quarkus.rest.client.reactive.runtime;
 
+import static org.jboss.resteasy.reactive.client.impl.RestClientRequestContext.INVOKED_EXCEPTION_MAPPER_CLASS_NAME_PROP;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -51,6 +53,7 @@ public void filter(ClientRequestContext requestContext, ClientResponseContext re
                     } else {
                         throwable = exceptionMapper.toThrowable(response);
                     }
+                    requestContext.setProperty(INVOKED_EXCEPTION_MAPPER_CLASS_NAME_PROP, exceptionMapper.getClass().getName());
                     if (throwable != null) {
                         throw new UnwrappableException(throwable);
                     }

independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java

@@ -58,11 +58,14 @@ public class RestClientRequestContext extends AbstractResteasyReactiveContext<Re
 
     public static final String INVOKED_METHOD_PROP = "org.eclipse.microprofile.rest.client.invokedMethod";
     public static final String INVOKED_METHOD_PARAMETERS_PROP = "io.quarkus.rest.client.invokedMethodParameters";
+    public static final String INVOKED_EXCEPTION_MAPPER_CLASS_NAME_PROP = "io.quarkus.rest.client.invokedExceptionMapperClass";
     public static final String DEFAULT_CONTENT_TYPE_PROP = "io.quarkus.rest.client.defaultContentType";
     public static final String DEFAULT_USER_AGENT_VALUE = "Quarkus REST Client";
     private static final String TMP_FILE_PATH_KEY = "tmp_file_path";
 
     static final MediaType IGNORED_MEDIA_TYPE = new MediaType("ignored", "ignored");
+    // TODO: the following property should really be provided by an SPI
+    private static final String DEFAULT_EXCEPTION_MAPPER_CLASS_NAME = "io.quarkus.rest.client.reactive.runtime.DefaultMicroprofileRestClientExceptionMapper";
 
     private final HttpClient httpClient;
     // Changeable by the request filter
@@ -179,12 +182,19 @@ public Annotation[] getMethodDeclaredAnnotationsSafe() {
     @Override
     protected Throwable unwrapException(Throwable t) {
         var res = super.unwrapException(t);
-        if (res instanceof WebApplicationException) {
-            var webApplicationException = (WebApplicationException) res;
+
+        var invokedExceptionMapperClassNameObj = properties.get(INVOKED_EXCEPTION_MAPPER_CLASS_NAME_PROP);
+        if (invokedExceptionMapperClassNameObj instanceof String invokedExceptionMapperClassName) {
+            if (!DEFAULT_EXCEPTION_MAPPER_CLASS_NAME.equals(invokedExceptionMapperClassName)) {
+                // in this case a custom exception mapper provided the exception, so we honor it
+                return res;
+            }
+        }
+
+        if (res instanceof WebApplicationException webApplicationException) {
             var message = webApplicationException.getMessage();
             var invokedMethodObject = properties.get(INVOKED_METHOD_PROP);
-            if ((invokedMethodObject instanceof Method) && !disableContextualErrorMessages) {
-                var invokedMethod = (Method) invokedMethodObject;
+            if ((invokedMethodObject instanceof Method invokedMethod) && !disableContextualErrorMessages) {
                 message = "Received: '" + message + "' when invoking: Rest Client method: '"
                         + invokedMethod.getDeclaringClass().getName() + "#"
                         + invokedMethod.getName() + "'";