Skip to content

Commit

Permalink
Decode errors during deserialization (#113)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ellisjoe
ellisjoe committed May 29, 2019
1 parent 2009c8a commit b0f7900
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 11 deletions.
20 changes: 18 additions & 2 deletions dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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);
}
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand Down
29 changes: 29 additions & 0 deletions ...ue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -103,6 +116,22 @@ public void testResponseUnknownContentType() throws IOException {
assertThat(body.contentType()).isEqualTo(json.getContentType());
}

@Test
public void testErrorsDecoded() {
TestResponse response = new TestResponse();
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 {

Expand Down
17 changes: 8 additions & 9 deletions ...serde/src/test/java/com/palantir/conjure/java/dialogue/serde/DefaultErrorDecoderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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:");
Expand All @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions dialogue-target/src/main/java/com/palantir/dialogue/ErrorDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit b0f7900

Please sign in to comment.