SynchronousMethodHandler does not wrap exceptions thrown by Decoder#decode in decode404 mode

Robert Fink · Robert Fink · commit 544820c9f4c2 · 2015-11-02T22:09:52.000-05:00
Removing exception wrapping adds flexibility to the interplay of Decoder and ErrorDecoder:
A Decoder can now delegate to an ErrorDecoder and the original ErrorDecoder exception gets
thrown rather than a Feign-specific DecodeException. An example use-case is a Jackson/Gson
implementation of special 404-handling of Optional&lt;Foo&gt; methods: when the Feign client has
decode404() enabled, then the Decoder is in charge of dispatching 404 to Optional#absent
where applicable, or to a delegate ErrorDecoder otherwise; consistent exception handling
requires that the exception produced by the ErrorDecoder does not wrapped.
diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java
@@ -121,7 +121,7 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
           return decode(response);
         }
       } else if (decode404 && response.status() == 404) {
-        return decode(response);
+        return decoder.decode(response, metadata.returnType());
       } else {
         throw errorDecoder.decode(metadata.configKey(), response);
       }
diff --git a/core/src/main/java/feign/codec/Decoder.java b/core/src/main/java/feign/codec/Decoder.java
@@ -18,6 +18,7 @@
 import java.io.IOException;
 import java.lang.reflect.Type;
 
+import feign.Feign;
 import feign.FeignException;
 import feign.Response;
 import feign.Util;
@@ -49,6 +50,9 @@
  * feign.Target#type() interface} processed by {@link feign.Feign#newInstance(feign.Target)}.  When
  * writing your implementation of Decoder, ensure you also test parameterized types such as {@code
  * List<Foo>}.
+ * <br/> <h3>Note on exception propagation</h3> Exceptions thrown by {@link Decoder}s get wrapped in
+ * a {@link DecodeException} unless they are a subclass of {@link FeignException} already, and unless
+ * the client was configured with {@link Feign.Builder#decode404()}.
  */
 public interface Decoder {
 
diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java
@@ -36,6 +36,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.concurrent.atomic.AtomicReference;
 
 import feign.Target.HardCodedTarget;
@@ -387,6 +388,23 @@ public Object decode(Response response, Type type) throws IOException {
     api.post();
   }
 
+  @Test
+  public void decoderCanThrowUnwrappedExceptionInDecode404Mode() throws Exception {
+    server.enqueue(new MockResponse().setResponseCode(404));
+    thrown.expect(NoSuchElementException.class);
+
+    TestInterface api = new TestInterfaceBuilder()
+        .decode404()
+        .decoder(new Decoder() {
+          @Override
+          public Object decode(Response response, Type type) throws IOException {
+            assertEquals(404, response.status());
+            throw new NoSuchElementException();
+          }
+        }).target("http://localhost:" + server.getPort());
+    api.post();
+  }
+
   @Test
   public void okIfEncodeRootCauseHasNoMessage() throws Exception {
     server.enqueue(new MockResponse().setBody("success!"));
@@ -594,6 +612,11 @@ TestInterfaceBuilder errorDecoder(ErrorDecoder errorDecoder) {
       return this;
     }
 
+    TestInterfaceBuilder decode404() {
+      delegate.decode404();
+      return this;
+    }
+
     TestInterface target(String url) {
       return delegate.target(TestInterface.class, url);
     }

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {`
`121`	`121`	`return decode(response);`
`122`	`122`	`}`
`123`	`123`	`} else if (decode404 && response.status() == 404) {`
`124`		`- return decode(response);`
	`124`	`+ return decoder.decode(response, metadata.returnType());`
`125`	`125`	`} else {`
`126`	`126`	`throw errorDecoder.decode(metadata.configKey(), response);`
`127`	`127`	`}`