From 5a312edc12cecf1a0eb997e378816fe2103d8e29 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 --- CHANGES.md | 3 ++ .../src/main/java/feign/MethodHandler.java | 2 +- .../main/java/feign/codec/ErrorDecoder.java | 31 ++++++++----------- feign-core/src/test/java/feign/FeignTest.java | 7 ++--- .../feign/codec/DefaultErrorDecoderTest.java | 6 ++-- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1f5d7de19d..6d663579ee 100644 --- a/CHANGES.md +++ b/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 diff --git a/feign-core/src/main/java/feign/MethodHandler.java b/feign-core/src/main/java/feign/MethodHandler.java index 598c2cf8c9..0e195e9e9b 100644 --- a/feign-core/src/main/java/feign/MethodHandler.java +++ b/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); diff --git a/feign-core/src/main/java/feign/codec/ErrorDecoder.java b/feign-core/src/main/java/feign/codec/ErrorDecoder.java index 73a05a3096..f0093425f2 100644 --- a/feign-core/src/main/java/feign/codec/ErrorDecoder.java +++ b/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.
+ * Allows you to massage an exception into a application-specific one. Converting out to a throttle + * exception are examples of this in use.
* Ex.
* * 
  * 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);
+ *   }
  *
  * }
  *
@@ -56,21 +54,18 @@ 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() { @@ -78,12 +73,12 @@ public interface 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 064e7dacc7..c6b5c9211e 100644 --- a/feign-core/src/test/java/feign/FeignTest.java +++ b/feign-core/src/test/java/feign/FeignTest.java @@ -112,10 +112,9 @@ Map 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); } }); } diff --git a/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java b/feign-core/src/test/java/feign/codec/DefaultErrorDecoderTest.java index 0325bdb533..75d9a6325b 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.create( 500, "Internal server error", ImmutableMap.>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.>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); } }