From 67148355c3625bd4d0aa1cd4f15063e30e670f61 Mon Sep 17 00:00:00 2001 From: adriancole Date: Sun, 7 Jul 2013 08:10:25 -0700 Subject: [PATCH] issue #9: fallback handling is differs between sync and observer responses; decouple from error handling --- .../src/main/java/feign/MethodHandler.java | 2 +- .../main/java/feign/codec/ErrorDecoder.java | 37 ++++++++----------- feign-core/src/test/java/feign/FeignTest.java | 6 +-- .../feign/codec/DefaultErrorDecoderTest.java | 6 +-- 4 files changed, 22 insertions(+), 29 deletions(-) diff --git a/feign-core/src/main/java/feign/MethodHandler.java b/feign-core/src/main/java/feign/MethodHandler.java index 149addf91e..7f6d9327db 100644 --- a/feign-core/src/main/java/feign/MethodHandler.java +++ b/feign-core/src/main/java/feign/MethodHandler.java @@ -111,7 +111,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); diff --git a/feign-core/src/main/java/feign/codec/ErrorDecoder.java b/feign-core/src/main/java/feign/codec/ErrorDecoder.java index 08a75faaf0..169935042a 100644 --- a/feign-core/src/main/java/feign/codec/ErrorDecoder.java +++ b/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.SECONDS; /** - * 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 + * Allows you to massage an exception into a application-specific one. Converting out to a throttle * exception are examples of this in use. *
* Ex. @@ -45,12 +42,12 @@ * 
  * class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
  *
- *     @Override
- *     public Object decode(String methodKey, Response response, Type<?> type) throws Throwable {
+ *   @Override
+ *   public Exception decode(String methodKey, Response response) {
  *    if (response.status() == 404)
  *        throw new IllegalArgumentException("zone not found");
- *    return ErrorDecoder.DEFAULT.decode(request, response, type);
- *     }
+ *    return ErrorDecoder.DEFAULT.decode(methodKey, request, response);
+ *   }
  *
  * }
  *
@@ -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 - * application-specific exceptions or return fallback values where possible. - * If your exception is retryable, wrap or subclass - * {@link RetryableException} + * {@link 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 application-specific exception decoded by the - * implementation. If the throwable is retryable, it should be - * wrapped, or a subtype of {@link RetryableException} + * @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 firstOrNull(Map> map, String key) { diff --git a/feign-core/src/test/java/feign/FeignTest.java b/feign-core/src/test/java/feign/FeignTest.java index 306de900fc..fd060f8a08 100644 --- a/feign-core/src/test/java/feign/FeignTest.java +++ b/feign-core/src/test/java/feign/FeignTest.java @@ -93,10 +93,10 @@ public void canOverrideErrorDecoderOnMethod() throws IOException, InterruptedExc return ImmutableMap.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 ErrorDecoder.DEFAULT.decode(methodKey, response, type); + return new IllegalArgumentException("zone not found"); + return ErrorDecoder.DEFAULT.decode(methodKey, response); } }); diff --git a/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java b/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java index 835b50c7af..bd3b17835f 100644 --- a/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java +++ b/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java @@ -34,7 +34,7 @@ public void throwsFeignException() throws Throwable { Response response = Response.create(500, "Internal server error", ImmutableMap.>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 void throwsFeignExceptionIncludingBody() throws Throwable { Response response = Response.create(500, "Internal server error", ImmutableMap.>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 void retryAfterHeaderThrowsRetryableException() throws Throwable { 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); } }