Skip to content

Commit 46be6bd

Browse files
author
adriancole
committed
issue OpenFeign#9: fallback handling is differs between sync and observer responses; decouple from error handling
1 parent 232c974 commit 46be6bd

5 files changed

Lines changed: 25 additions & 29 deletions

File tree

‎CHANGES.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
### Version 3.0
2+
* decoupled ErrorDecoder from fallback handling
3+
14
### Version 2.0.0
25
* removes guava and jax-rs dependencies
36
* adds JAX-RS integration

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public Object executeAndDecode(String configKey, RequestTemplate template, Type
111111
}
112112
return decoder.decode(configKey, response, returnType);
113113
} else {
114-
return errorDecoder.decode(configKey, response, returnType);
114+
throw errorDecoder.decode(configKey, response);
115115
}
116116
} catch (Throwable e) {
117117
ensureBodyClosed(response);

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

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package feign.codec;
1717

18-
import java.lang.reflect.Type;
1918
import java.text.DateFormat;
2019
import java.text.ParseException;
2120
import java.text.SimpleDateFormat;
@@ -35,22 +34,20 @@
3534
import static java.util.concurrent.TimeUnit.SECONDS;
3635

3736
/**
38-
* Allows you to massage an exception into a application-specific one, or
39-
* fallback to a default value. Falling back to null on
40-
* {@link Response#status() status 404}, or converting out to a throttle
37+
* Allows you to massage an exception into a application-specific one. Converting out to a throttle
4138
* exception are examples of this in use.
4239
* <br>
4340
* Ex.
4441
* <br>
4542
* <pre>
4643
* class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
4744
*
48-
* &#064;Override
49-
* public Object decode(String methodKey, Response response, Type&lt;?&gt; type) throws Throwable {
45+
* &#064;Override
46+
* public Exception decode(String methodKey, Response response) {
5047
* if (response.status() == 404)
5148
* throw new IllegalArgumentException(&quot;zone not found&quot;);
52-
* return ErrorDecoder.DEFAULT.decode(request, response, type);
53-
* }
49+
* return ErrorDecoder.DEFAULT.decode(methodKey, request, response);
50+
* }
5451
*
5552
* }
5653
* </pre>
@@ -59,33 +56,29 @@ public interface ErrorDecoder {
5956

6057
/**
6158
* Implement this method in order to decode an HTTP {@link Response} when
62-
* {@link Response#status()} is not in the 2xx range. Please raise
63-
* application-specific exceptions or return fallback values where possible.
64-
* If your exception is retryable, wrap or subclass
65-
* {@link RetryableException}
59+
* {@link Response#status()} is not in the 2xx range. Please raise application-specific exceptions where possible.
60+
* If your exception is retryable, wrap or subclass {@link RetryableException}
6661
*
6762
* @param methodKey {@link feign.Feign#configKey} of the java method that invoked the request. ex. {@code IAM#getUser()}
6863
* @param response HTTP response where {@link Response#status() status} is greater than or equal to {@code 300}.
69-
* @param type Target object type.
70-
* @return instance of {@code type}
71-
* @throws Throwable IOException, if there was a network error reading the
72-
* response or an application-specific exception decoded by the
73-
* implementation. If the throwable is retryable, it should be
74-
* wrapped, or a subtype of {@link RetryableException}
64+
* @return Exception IOException, if there was a network error reading the
65+
* response or an application-specific exception decoded by the
66+
* implementation. If the throwable is retryable, it should be
67+
* wrapped, or a subtype of {@link RetryableException}
7568
*/
76-
public Object decode(String methodKey, Response response, Type type) throws Throwable;
69+
public Exception decode(String methodKey, Response response);
7770

7871
public static final ErrorDecoder DEFAULT = new ErrorDecoder() {
7972

8073
private final RetryAfterDecoder retryAfterDecoder = new RetryAfterDecoder();
8174

8275
@Override
83-
public Object decode(String methodKey, Response response, Type type) throws Throwable {
76+
public Exception decode(String methodKey, Response response) {
8477
FeignException exception = errorStatus(methodKey, response);
8578
Date retryAfter = retryAfterDecoder.apply(firstOrNull(response.headers(), RETRY_AFTER));
8679
if (retryAfter != null)
87-
throw new RetryableException(exception.getMessage(), exception, retryAfter);
88-
throw exception;
80+
return new RetryableException(exception.getMessage(), exception, retryAfter);
81+
return exception;
8982
}
9083

9184
private <T> T firstOrNull(Map<String, Collection<T>> map, String key) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ public void canOverrideErrorDecoderOnMethod() throws IOException, InterruptedExc
9393
return ImmutableMap.<String, ErrorDecoder>of("TestInterface#post()", new ErrorDecoder() {
9494

9595
@Override
96-
public Object decode(String methodKey, Response response, Type type) throws Throwable {
96+
public Exception decode(String methodKey, Response response) {
9797
if (response.status() == 404)
98-
throw new IllegalArgumentException("zone not found");
99-
return ErrorDecoder.DEFAULT.decode(methodKey, response, type);
98+
return new IllegalArgumentException("zone not found");
99+
return ErrorDecoder.DEFAULT.decode(methodKey, response);
100100
}
101101

102102
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,22 @@ public void throwsFeignException() throws Throwable {
3434
Response response = Response.create(500, "Internal server error", ImmutableMap.<String, Collection<String>>of(),
3535
null);
3636

37-
ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
37+
throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
3838
}
3939

4040
@Test(expectedExceptions = FeignException.class, expectedExceptionsMessageRegExp = "status 500 reading Service#foo\\(\\); content:\nhello world")
4141
public void throwsFeignExceptionIncludingBody() throws Throwable {
4242
Response response = Response.create(500, "Internal server error", ImmutableMap.<String, Collection<String>>of(),
4343
"hello world");
4444

45-
ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
45+
throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
4646
}
4747

4848
@Test(expectedExceptions = RetryableException.class, expectedExceptionsMessageRegExp = "status 503 reading Service#foo\\(\\)")
4949
public void retryAfterHeaderThrowsRetryableException() throws Throwable {
5050
Response response = Response.create(503, "Service Unavailable",
5151
ImmutableMultimap.of(RETRY_AFTER, "Sat, 1 Jan 2000 00:00:00 GMT").asMap(), null);
5252

53-
ErrorDecoder.DEFAULT.decode("Service#foo()", response, void.class);
53+
throw ErrorDecoder.DEFAULT.decode("Service#foo()", response);
5454
}
5555
}

0 commit comments

Comments
 (0)