Merge pull request #32715 from geoand/#32710

Throw better exception when REST Client receives invalid JSON
quarkusio · Apr 18, 2023 · d08daf1 · d08daf1
2 parents 89ca6e7 + e542cc9
commit d08daf1
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 2 deletions.
diff --git a/...src/test/java/io/quarkus/rest/client/reactive/jackson/test/InvalidJsonFromServerTest.java b/...src/test/java/io/quarkus/rest/client/reactive/jackson/test/InvalidJsonFromServerTest.java
@@ -0,0 +1,61 @@
+package io.quarkus.rest.client.reactive.jackson.test;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+
+import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;
+import org.eclipse.microprofile.rest.client.inject.RestClient;
+import org.jboss.resteasy.reactive.ClientWebApplicationException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+
+/**
+ * Tests that when the server responds with data that is not valid JSON, we return an internal server error
+ */
+public class InvalidJsonFromServerTest {
+    @RegisterExtension
+    static final QuarkusUnitTest TEST = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(JsonObject.class, JsonClient.class, InvalidJsonEndpoint.class));
+
+    @RestClient
+    JsonClient client;
+
+    @Test
+    public void test() {
+        assertThatThrownBy(() -> client.get())
+                .isInstanceOf(ClientWebApplicationException.class)
+                .hasMessageContaining("HTTP 200")
+                .cause()
+                .hasMessageContaining("was expecting double-quote to start field name");
+    }
+
+    @Path("/invalid-json")
+    @RegisterRestClient(baseUri = "http://localhost:8081")
+    public interface JsonClient {
+
+        @Produces(MediaType.APPLICATION_JSON)
+        @GET
+        JsonObject get();
+    }
+
+    static class JsonObject {
+        public String name;
+    }
+
+    @Path("/invalid-json")
+    @Produces(MediaType.APPLICATION_JSON)
+    public static class InvalidJsonEndpoint {
+
+        @GET
+        public String get() {
+            return "{name: test}";
+        }
+    }
+}
diff --git a/...rkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java b/...rkus/rest/client/reactive/jackson/runtime/serialisers/ClientJacksonMessageBodyReader.java
@@ -14,18 +14,22 @@
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
 
+import org.jboss.logging.Logger;
 import org.jboss.resteasy.reactive.ClientWebApplicationException;
 import org.jboss.resteasy.reactive.client.impl.RestClientRequestContext;
 import org.jboss.resteasy.reactive.client.spi.ClientRestHandler;
 import org.jboss.resteasy.reactive.server.jackson.JacksonBasicMessageBodyReader;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.exc.StreamReadException;
 import com.fasterxml.jackson.databind.DatabindException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectReader;
 
 public class ClientJacksonMessageBodyReader extends JacksonBasicMessageBodyReader implements ClientRestHandler {
 
+    private static final Logger log = Logger.getLogger(ClientJacksonMessageBodyReader.class);
+
     private final ConcurrentMap<ObjectMapper, ObjectReader> contextResolverMap = new ConcurrentHashMap<>();
     private RestClientRequestContext context;
 
@@ -39,8 +43,11 @@ public Object readFrom(Class<Object> type, Type genericType, Annotation[] annota
             MultivaluedMap<String, String> httpHeaders, InputStream entityStream) throws IOException, WebApplicationException {
         try {
             return super.readFrom(type, genericType, annotations, mediaType, httpHeaders, entityStream);
+        } catch (JsonParseException e) {
+            log.debug("Server returned invalid json data", e);
+            throw new ClientWebApplicationException(e, Response.Status.OK);
         } catch (StreamReadException | DatabindException e) {
-            throw new ClientWebApplicationException(e, Response.Status.BAD_REQUEST);
+            throw new ClientWebApplicationException(e, Response.Status.BAD_REQUEST); // TODO: we need to check if this actually makes sense...
         }
     }
 

diff --git a/...ntime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java b/...ntime/src/main/java/org/jboss/resteasy/reactive/client/impl/RestClientRequestContext.java
@@ -179,7 +179,9 @@ protected Throwable unwrapException(Throwable t) {
                         + invokedMethod.getDeclaringClass().getName() + "#"
                         + invokedMethod.getName() + "'";
             }
-            return new ClientWebApplicationException(message, webApplicationException,
+            return new ClientWebApplicationException(message,
+                    webApplicationException instanceof ClientWebApplicationException ? webApplicationException.getCause()
+                            : webApplicationException,
                     webApplicationException.getResponse());
         }
         return res;