issue #9: fallback handling is differs between sync and observer resp…

…onses; 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

@@ -122,7 +122,7 @@ public Object executeAndDecode(String configKey, RequestTemplate template, Type
         }
         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

@@ -25,7 +25,6 @@
 import feign.FeignException;
 import feign.Response;
 import feign.RetryableException;
-import java.lang.reflect.Type;
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
@@ -34,20 +33,19 @@
 import java.util.Map;
 
 /**
- * Allows you to massage an exception into a application-specific one, or 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>
+ * Allows you to massage an exception into a application-specific one. Converting out to a throttle
+ * exception are examples of this in use. <br>
  * Ex. <br>
  *
  * <pre>
  * class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
  *
- *     &#064;Override
- *     public Object decode(String methodKey, Response response, Type&lt;?&gt; type) throws Throwable {
+ *   &#064;Override
+ *   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>
@@ -56,34 +54,31 @@ 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 application-specific exceptions or
-   * return fallback values where possible. If your exception is retryable, wrap or subclass {@link
-   * RetryableException}
+   * Response#status()} is not in the 2xx range. Please raise application-specific exceptions where
+   * possible. 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 instance of {@code type}
-   * @throws Throwable IOException, if there was a network error reading the response or an
+   * @return Exception IOException, if there was a network error reading the response or an
    *     application-specific exception decoded by the 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);
-          throw exception;
+            return new RetryableException(exception.getMessage(), exception, retryAfter);
+          return exception;
         }
 
         private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {

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

@@ -112,10 +112,9 @@ Map<String, ErrorDecoder> decoders() {
             new ErrorDecoder() {
 
               @Override
-              public Object decode(String methodKey, Response response, Type type)
-                  throws Throwable {
-                if (response.status() == 404) throw new IllegalArgumentException("zone not found");
-                return ErrorDecoder.DEFAULT.decode(methodKey, response, type);
+              public Exception decode(String methodKey, Response response) {
+                if (response.status() == 404) return new IllegalArgumentException("zone not found");
+                return ErrorDecoder.DEFAULT.decode(methodKey, response);
               }
             });
       }

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

@@ -34,7 +34,7 @@ public void throwsFeignException() throws Throwable {
         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(
@@ -49,7 +49,7 @@ public void throwsFeignExceptionIncludingBody() throws Throwable {
             ImmutableMap.<String, Collection<String>>of(),
             "hello world");
 
-    ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
+    throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
   }
 
   @Test(
@@ -63,6 +63,6 @@ public void retryAfterHeaderThrowsRetryableException() throws Throwable {
             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);
   }
 }