Decode errors during deserialization

dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java

@@ -16,10 +16,12 @@
 
 package com.palantir.conjure.java.dialogue.serde;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.palantir.dialogue.BinaryRequestBody;
 import com.palantir.dialogue.BodySerDe;
 import com.palantir.dialogue.Deserializer;
+import com.palantir.dialogue.ErrorDecoder;
 import com.palantir.dialogue.Headers;
 import com.palantir.dialogue.RequestBody;
 import com.palantir.dialogue.Response;
@@ -42,6 +44,7 @@ final class ConjureBodySerDe implements BodySerDe {
     private static final String BINARY_CONTENT_TYPE = "application/octet-stream";
 
     private final List<Encoding> encodings;
+    private final ErrorDecoder errorDecoder;
     private final Encoding defaultEncoding;
 
     /**
@@ -50,8 +53,15 @@ final class ConjureBodySerDe implements BodySerDe {
      * by a given request, or the first serializer if no such serializer can be found.
      */
     ConjureBodySerDe(List<Encoding> encodings) {
+        // TODO(jellis): consider supporting cbor encoded errors
+        this(encodings, DefaultErrorDecoder.INSTANCE);
+    }
+
+    @VisibleForTesting
+    ConjureBodySerDe(List<Encoding> encodings, ErrorDecoder errorDecoder) {
         // Defensive copy
         this.encodings = ImmutableList.copyOf(encodings);
+        this.errorDecoder = errorDecoder;
         Preconditions.checkArgument(encodings.size() > 0, "At least one Encoding is required");
         this.defaultEncoding = encodings.get(0);
     }
@@ -63,7 +73,7 @@ public <T> Serializer<T> serializer(TypeMarker<T> token) {
 
     @Override
     public <T> Deserializer<T> deserializer(TypeMarker<T> token) {
-        return new EncodingDeserializerRegistry<>(encodings, token);
+        return new EncodingDeserializerRegistry<>(encodings, errorDecoder, token);
     }
 
     @Override
@@ -168,15 +178,21 @@ private static final class EncodingSerializerContainer<T> {
     private static final class EncodingDeserializerRegistry<T> implements Deserializer<T> {
 
         private final List<EncodingDeserializerContainer<T>> encodings;
+        private final ErrorDecoder errorDecoder;
 
-        EncodingDeserializerRegistry(List<Encoding> encodings, TypeMarker<T> token) {
+        EncodingDeserializerRegistry(List<Encoding> encodings, ErrorDecoder errorDecoder, TypeMarker<T> token) {
             this.encodings = encodings.stream()
                     .map(encoding -> new EncodingDeserializerContainer<>(encoding, token))
                     .collect(ImmutableList.toImmutableList());
+            this.errorDecoder = errorDecoder;
         }
 
         @Override
         public T deserialize(Response response) {
+            if (errorDecoder.isError(response)) {
+                throw errorDecoder.decode(response);
+            }
+
             Optional<String> contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE);
             EncodingDeserializerContainer<T> container = getResponseDeserializer(contentType);
             return container.deserializer.deserialize(response.body());

dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java

@@ -17,11 +17,18 @@
 package com.palantir.conjure.java.dialogue.serde;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.palantir.conjure.java.api.errors.ErrorType;
+import com.palantir.conjure.java.api.errors.RemoteException;
+import com.palantir.conjure.java.api.errors.SerializableError;
+import com.palantir.conjure.java.api.errors.ServiceException;
 import com.palantir.dialogue.BodySerDe;
+import com.palantir.dialogue.ErrorDecoder;
 import com.palantir.dialogue.RequestBody;
 import com.palantir.dialogue.Response;
 import com.palantir.dialogue.TypeMarker;
@@ -35,11 +42,17 @@
 import java.util.Map;
 import javax.ws.rs.core.HttpHeaders;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
 
+@RunWith(MockitoJUnitRunner.class)
 public class ConjureBodySerDeTest {
 
     private static final TypeMarker<String> TYPE = new TypeMarker<String>() {};
 
+    @Mock private ErrorDecoder errorDecoder;
+
     @Test
     public void testRequestContentType() throws IOException {
         Encoding json = new StubEncoding("application/json");
@@ -103,6 +116,23 @@ public void testResponseUnknownContentType() throws IOException {
         assertThat(body.contentType()).isEqualTo(json.getContentType());
     }
 
+    @Test
+    public void testErrorsDecoded() {
+        TestResponse response = new TestResponse();
+        // response.contentType();
+        response.code = 400;
+
+        ServiceException serviceException = new ServiceException(ErrorType.INVALID_ARGUMENT);
+        SerializableError serialized = SerializableError.forException(serviceException);
+        when(errorDecoder.isError(response)).thenReturn(true);
+        when(errorDecoder.decode(response)).thenReturn(new RemoteException(serialized, 400));
+
+        BodySerDe serializers = new ConjureBodySerDe(ImmutableList.of(new StubEncoding("text/plain")), errorDecoder);
+
+        assertThatExceptionOfType(RemoteException.class)
+                .isThrownBy(() -> serializers.deserializer(TYPE).deserialize(response));
+    }
+
     /** Deserializes requests as the configured content type. */
     public static final class StubEncoding implements Encoding {
 

dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultErrorDecoderTest.java

@@ -68,7 +68,10 @@ private static String createServiceException(ServiceException exception) {
     @Test
     public void extractsRemoteExceptionForAllErrorCodes() {
         for (int code : ImmutableList.of(300, 400, 404, 500)) {
-            RemoteException exception = decode("application/json", code, SERIALIZED_EXCEPTION);
+            Response response = response(code, "application/json", SERIALIZED_EXCEPTION);
+            assertThat(decoder.isError(response)).isTrue();
+
+            RemoteException exception = decoder.decode(response);
             assertThat(exception.getCause()).isNull();
             assertThat(exception.getStatus()).isEqualTo(code);
             assertThat(exception.getError().errorCode()).isEqualTo(ErrorType.FAILED_PRECONDITION.code().name());
@@ -89,21 +92,21 @@ public void testSpecificException() {
 
     @Test
     public void cannotDecodeNonJsonMediaTypes() {
-        assertThatThrownBy(() -> decode("text/plain", 500, SERIALIZED_EXCEPTION))
+        assertThatThrownBy(() -> decoder.decode(response(500, "text/plain", SERIALIZED_EXCEPTION)))
                 .isInstanceOf(SafeRuntimeException.class)
                 .hasMessage("Failed to interpret response body as SerializableError: {code=500}");
     }
 
     @Test
     public void doesNotHandleUnparseableBody() {
-        assertThatThrownBy(() -> decode("application/json/", 500, "not json"))
+        assertThatThrownBy(() -> decoder.decode(response(500, "application/json/", "not json")))
                 .isInstanceOf(SafeRuntimeException.class)
                 .hasMessageStartingWith("Failed to interpret response body as SerializableError:");
     }
 
     @Test
     public void doesNotHandleNullBody() {
-        assertThatThrownBy(() -> decode("application/json", 500, null))
+        assertThatThrownBy(() -> decoder.decode(response(500, "application/json", null)))
                 .isInstanceOf(SafeRuntimeException.class)
                 .hasMessageStartingWith(
                         "Failed to deserialize response body as JSON, could not deserialize SerializableError:");
@@ -127,11 +130,7 @@ private static RemoteException encodeAndDecode(Exception exception) {
         //         ? ((WebApplicationException) exception).getResponse().getStatus()
         //         : 400;
         int status = 400;
-        return decode("application/json", status, json);
-    }
-
-    private static RemoteException decode(String contentType, int status, @CheckForNull String body) {
-        return decoder.decode(response(status, contentType, body));
+        return decoder.decode(response(status, "application/json", json));
     }
 
     private static Response response(int code, String mediaType, @CheckForNull String body) {

dialogue-target/src/main/java/com/palantir/dialogue/ErrorDecoder.java

@@ -25,5 +25,9 @@
  * Response} does not adhere to an expected format.
  */
 public interface ErrorDecoder {
+    default boolean isError(Response response) {
+        return 300 <= response.code() && response.code() <= 599;
+    }
+
     RemoteException decode(Response response);
 }