issue #9: fallback handling is differs between sync and observer responses; decouple from error handling

CHANGES.md

@@ -1,3 +1,6 @@
+### Version 3.0
+* decoupled ErrorDecoder from fallback handling
+
 ### Version 2.0.0
 * removes guava and jax-rs dependencies
 * adds JAX-RS integration

feign-core/src/main/java/feign/MethodHandler.java

@@ -111,7 +111,7 @@ final class MethodHandler {
         }
         return decoder.decode(configKey, response, returnType);
       } else {
-        return errorDecoder.decode(configKey, response, returnType);
+        throw errorDecoder.decode(configKey, response);
       }
     } catch (Throwable e) {
       ensureBodyClosed(response);

feign-core/src/main/java/feign/codec/ErrorDecoder.java

@@ -15,7 +15,6 @@
  */
 package feign.codec;
 
-import java.lang.reflect.Type;
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
@@ -35,9 +34,7 @@ import static java.util.concurrent.TimeUnit.NANOSECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 /**
- * Allows you to massage an exception into a application-specific one, or
+ * Allows you to massage an exception into a application-specific one. Converting out to a throttle
- * fallback to a default value. Falling back to null on
- * {@link Response#status() status 404}, or converting out to a throttle
  * exception are examples of this in use.
  * <br>
  * Ex.
@@ -45,12 +42,12 @@ import static java.util.concurrent.TimeUnit.SECONDS;
  * <pre>
  * class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
  *
- *     &#064;Override
+ *   &#064;Override
- *     public Object decode(String methodKey, Response response, Type&lt;?&gt; type) throws Throwable {
+ *   public Exception decode(String methodKey, Response response) {
  *    if (response.status() == 404)
  *        throw new IllegalArgumentException(&quot;zone not found&quot;);
- *    return ErrorDecoder.DEFAULT.decode(request, response, type);
+ *    return ErrorDecoder.DEFAULT.decode(methodKey, request, response);
- *     }
+ *   }
  *
  * }
  * </pre>
@@ -59,33 +56,29 @@ public interface ErrorDecoder {
 
   /**
    * Implement this method in order to decode an HTTP {@link Response} when
-   * {@link Response#status()} is not in the 2xx range. Please raise
+   * {@link Response#status()} is not in the 2xx range. Please raise  application-specific exceptions where possible.
-   * application-specific exceptions or return fallback values where possible.
+   * If your exception is retryable, wrap or subclass {@link RetryableException}
-   * If your exception is retryable, wrap or subclass
-   * {@link RetryableException}
    *
    * @param methodKey {@link feign.Feign#configKey} of the java method that invoked the request.  ex. {@code IAM#getUser()}
    * @param response  HTTP response where {@link Response#status() status} is greater than or equal to {@code 300}.
-   * @param type      Target object type.
+   * @return Exception IOException, if there was a network error reading the
-   * @return instance of {@code type}
+   *         response or an application-specific exception decoded by the
-   * @throws Throwable IOException, if there was a network error reading the
+   *         implementation. If the throwable is retryable, it should be
-   *                   response or an application-specific exception decoded by the
+   *         wrapped, or a subtype of {@link RetryableException}
-   *                   implementation. If the throwable is retryable, it should be
-   *                   wrapped, or a subtype of {@link RetryableException}
    */
-  public Object decode(String methodKey, Response response, Type type) throws Throwable;
+  public Exception decode(String methodKey, Response response);
 
   public static final ErrorDecoder DEFAULT = new ErrorDecoder() {
 
     private final RetryAfterDecoder retryAfterDecoder = new RetryAfterDecoder();
 
     @Override
-    public Object decode(String methodKey, Response response, Type type) throws Throwable {
+    public Exception decode(String methodKey, Response response) {
       FeignException exception = errorStatus(methodKey, response);
       Date retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
       if (retryAfter != null)
-        throw new RetryableException(exception.getMessage(), exception, retryAfter);
+        return new RetryableException(exception.getMessage(), exception, retryAfter);
-      throw exception;
+      return exception;
     }
 
     private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {

feign-core/src/test/java/feign/FeignTest.java

@@ -93,10 +93,10 @@ public class FeignTest {
         return ImmutableMap.<String, ErrorDecoder>of("TestInterface#post()", new ErrorDecoder() {
 
           @Override
-          public Object decode(String methodKey, Response response, Type type) throws Throwable {
+          public Exception decode(String methodKey, Response response) {
             if (response.status() == 404)
-              throw new IllegalArgumentException("zone not found");
+              return new IllegalArgumentException("zone not found");
-            return ErrorDecoder.DEFAULT.decode(methodKey, response, type);
+            return ErrorDecoder.DEFAULT.decode(methodKey, response);
           }
 
         });

feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java

@@ -34,7 +34,7 @@ public class DefaultErrorDecoderTest {
     Response response = Response.create(500, "Internal server error", ImmutableMap.<String, Collection<String>>of(),
         null);
 
-    ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
+    throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
   }
 
   @Test(expectedExceptions = FeignException.class, expectedExceptionsMessageRegExp = "status 500 reading Service#foo\\(\\); content:\nhello world")
@@ -42,7 +42,7 @@ public class DefaultErrorDecoderTest {
     Response response = Response.create(500, "Internal server error", ImmutableMap.<String, Collection<String>>of(),
         "hello world");
 
-    ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
+    throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
   }
 
   @Test(expectedExceptions = RetryableException.class, expectedExceptionsMessageRegExp = "status 503 reading Service#foo\\(\\)")
@@ -50,6 +50,6 @@ public class DefaultErrorDecoderTest {
     Response response = Response.create(503, "Service Unavailable",
         ImmutableMultimap.of(RETRY_AFTER, "Sat, 1 Jan 2000 00:00:00 GMT").asMap(), null);
 
-    ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
+    throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
   }
 }