Response is closed after decoder fails (OpenFeign#668)

Carter Kozak · velo · commit 85f2f5047932 · 2018-05-05T13:17:18.000+12:00
Previously a decoder failure resulting in an exception would
fail to close responses when "doNotCloseAfterDecode" was enabled.
diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java
@@ -131,12 +131,14 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
         if (void.class == metadata.returnType()) {
           return null;
         } else {
+          Object result = decode(response);
           shouldClose = closeAfterDecode;
-          return decode(response);
+          return result;
         }
       } else if (decode404 && response.status() == 404 && void.class != metadata.returnType()) {
+        Object result = decode(response);
         shouldClose = closeAfterDecode;
-        return decode(response);
+        return result;
       } else {
         throw errorDecoder.decode(metadata.configKey(), response);
       }
diff --git a/core/src/test/java/feign/FeignBuilderTest.java b/core/src/test/java/feign/FeignBuilderTest.java
@@ -21,6 +21,8 @@
 import org.junit.Test;
 
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
@@ -29,6 +31,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import feign.codec.Decoder;
@@ -349,6 +352,75 @@ public Object next() {
     assertEquals(1, server.getRequestCount());
   }
 
+  /**
+   * When {@link Feign.Builder#doNotCloseAfterDecode()} is enabled an an exception
+   * is thrown from the {@link Decoder}, the response should be closed.
+   */
+  @Test
+  public void testDoNotCloseAfterDecodeDecoderFailure() throws Exception {
+    server.enqueue(new MockResponse().setBody("success!"));
+
+    String url = "http://localhost:" + server.getPort();
+    Decoder angryDecoder = new Decoder() {
+      @Override
+      public Object decode(Response response, Type type) throws IOException {
+        throw new IOException("Failed to decode the response");
+      }
+    };
+
+    final AtomicBoolean closed = new AtomicBoolean();
+    TestInterface api = Feign.builder()
+            .client(new Client() {
+              Client client = new Client.Default(null, null);
+              @Override
+              public Response execute(Request request, Request.Options options) throws IOException {
+                final Response original = client.execute(request, options);
+                return Response.builder()
+                        .status(original.status())
+                        .headers(original.headers())
+                        .reason(original.reason())
+                        .request(original.request())
+                        .body(new Response.Body() {
+                          @Override
+                          public Integer length() {
+                            return original.body().length();
+                          }
+
+                          @Override
+                          public boolean isRepeatable() {
+                            return original.body().isRepeatable();
+                          }
+
+                          @Override
+                          public InputStream asInputStream() throws IOException {
+                            return original.body().asInputStream();
+                          }
+
+                          @Override
+                          public Reader asReader() throws IOException {
+                            return original.body().asReader();
+                          }
+
+                          @Override
+                          public void close() throws IOException {
+                            closed.set(true);
+                            original.body().close();
+                          }
+                        })
+                        .build();
+              }
+            })
+            .decoder(angryDecoder)
+            .doNotCloseAfterDecode()
+            .target(TestInterface.class, url);
+    try {
+      api.decodedLazyPost();
+      fail("Expected an exception");
+    } catch (FeignException expected) {
+    }
+    assertTrue("Responses must be closed when the decoder fails", closed.get());
+  }
+
   interface TestInterface {
     @RequestLine("GET")
     Response getNoPath();
