diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java index 7bfda2aa7..461c0958f 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java @@ -198,7 +198,7 @@ private static final class EncodingDeserializerRegistry implements Deserializ EncodingDeserializerRegistry(List encodings, ErrorDecoder errorDecoder, TypeMarker token) { this.encodings = encodings.stream() - .map(encoding -> EncodingDeserializerContainer.of(encoding, token)) + .map(encoding -> new EncodingDeserializerContainer<>(encoding, token)) .collect(ImmutableList.toImmutableList()); this.errorDecoder = errorDecoder; this.token = token; @@ -279,13 +279,9 @@ private static final class EncodingDeserializerContainer { private final Encoding encoding; private final Encoding.Deserializer deserializer; - EncodingDeserializerContainer(Encoding encoding, Encoding.Deserializer deserializer) { + EncodingDeserializerContainer(Encoding encoding, TypeMarker token) { this.encoding = encoding; - this.deserializer = deserializer; - } - - static EncodingDeserializerContainer of(Encoding encoding, TypeMarker token) { - return new EncodingDeserializerContainer<>(encoding, encoding.deserializer(token)); + this.deserializer = encoding.deserializer(token); } @Override diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java index 22880e4eb..f55fbd1da 100644 --- a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java +++ b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java @@ -30,8 +30,6 @@ import java.nio.charset.StandardCharsets; import java.util.Optional; import javax.ws.rs.core.HttpHeaders; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Extracts and returns a {@link RemoteException} from an {@link Response}. @@ -42,7 +40,6 @@ enum ErrorDecoder { INSTANCE; - private static final Logger log = LoggerFactory.getLogger(ErrorDecoder.class); private static final ObjectMapper MAPPER = ObjectMappers.newClientObjectMapper(); boolean isError(Response response) { @@ -53,40 +50,26 @@ RemoteException decode(Response response) { // TODO(rfink): What about HTTP/101 switching protocols? // TODO(rfink): What about HEAD requests? - InputStream inputStream = response.body(); + String body; try { - String body; - try { - body = toString(inputStream); - } catch (NullPointerException | IOException e) { - UnknownRemoteException exception = new UnknownRemoteException(response.code(), ""); - exception.initCause(e); - throw exception; - } - - Optional contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE); - if (contentType.isPresent() && Encodings.matchesContentType("application/json", contentType.get())) { - try { - SerializableError serializableError = MAPPER.readValue(body, SerializableError.class); - return new RemoteException(serializableError, response.code()); - } catch (Exception e) { - throw new UnknownRemoteException(response.code(), body); - } - } + body = toString(response.body()); + } catch (NullPointerException | IOException e) { + UnknownRemoteException exception = new UnknownRemoteException(response.code(), ""); + exception.initCause(e); + throw exception; + } - throw new UnknownRemoteException(response.code(), body); - } finally { - try { - inputStream.close(); - } catch (RuntimeException | IOException e) { - log.warn("Failed to close InputStream", e); - } + Optional contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE); + if (contentType.isPresent() && Encodings.matchesContentType("application/json", contentType.get())) { try { - response.close(); - } catch (RuntimeException e) { - log.warn("Failed to close Response", e); + SerializableError serializableError = MAPPER.readValue(body, SerializableError.class); + return new RemoteException(serializableError, response.code()); + } catch (Exception e) { + throw new UnknownRemoteException(response.code(), body); } } + + throw new UnknownRemoteException(response.code(), body); } private static String toString(InputStream body) throws IOException { diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java index 4d0b329d5..dcbdd6bd7 100644 --- a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java +++ b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java @@ -33,7 +33,6 @@ import com.palantir.conjure.java.api.errors.UnknownRemoteException; import com.palantir.conjure.java.serialization.ObjectMappers; import com.palantir.dialogue.Response; -import com.palantir.dialogue.TestResponse; import com.palantir.logsafe.Preconditions; import com.palantir.logsafe.SafeArg; import java.io.ByteArrayInputStream; @@ -147,18 +146,6 @@ public void handlesJsonWithEncoding() { assertThat(exception.getError().errorName()).isEqualTo(ErrorType.FAILED_PRECONDITION.name()); } - @Test - public void closes_response_and_inputstream() { - TestResponse testResponse = new TestResponse().contentType("application/json"); - assertThatThrownBy(() -> decoder.decode(testResponse)).isInstanceOf(UnknownRemoteException.class); - assertThat(testResponse.body().isClosed()) - .describedAs("Expected inputstream to be closed") - .isTrue(); - assertThat(testResponse.isClosed()) - .describedAs("Body should probably be closed too") - .isTrue(); - } - private static RemoteException encodeAndDecode(Exception exception) { Preconditions.checkArgument(!(exception instanceof ServiceException), "Use SerializableError#forException"); Object error = SerializableError.builder()