Skip to content

Commit 4b2a48e

Browse files
dinomitevelo
dinomite
authored and
velo
committed
Unwrap RetryableException and throw cause (OpenFeign#737)
* Throw cause of RetryableExceptions * Allow propogation of underlying exceptions Add configuration to Feign.Builder and support in SynchronousMethodHandler to make it propagate the cause of RetryableExceptions * Retab SMH * Add note about propagation in readme * Use enum for exception propagation policy
1 parent 6b608d0 commit 4b2a48e

6 files changed

Lines changed: 109 additions & 10 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,7 +751,9 @@ public class Example {
751751
`Retryer`s are responsible for determining if a retry should occur by returning either a `true` or
752752
`false` from the method `continueOrPropagate(RetryableException e);` A `Retryer` instance will be
753753
created for each `Client` execution, allowing you to maintain state bewteen each request if desired.
754-
If the retry is determined to be unsucessful, the last `RetryException` will be thrown.
754+
755+
If the retry is determined to be unsuccessful, the last `RetryException` will be thrown. To throw the original
756+
cause that led to the unsuccessful retry, build your Feign client with the `exceptionPropagationPolicy()` option.
755757

756758
#### Static and Default Methods
757759
Interfaces targeted by Feign may have static or default methods (if using Java 8+).

‎core/src/main/java/feign/ExceptionPropagationPolicy.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Copyright 2012-2018 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign;
15+
16+
public enum ExceptionPropagationPolicy {
17+
NONE, UNWRAP
18+
}

‎core/src/main/java/feign/Feign.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import feign.codec.Decoder;
2626
import feign.codec.Encoder;
2727
import feign.codec.ErrorDecoder;
28+
import static feign.ExceptionPropagationPolicy.NONE;
2829

2930
/**
3031
* Feign's purpose is to ease development against http apis that feign restfulness. <br>
@@ -109,6 +110,7 @@ public static class Builder {
109110
new InvocationHandlerFactory.Default();
110111
private boolean decode404;
111112
private boolean closeAfterDecode = true;
113+
private ExceptionPropagationPolicy propagationPolicy = NONE;
112114

113115
public Builder logLevel(Logger.Level logLevel) {
114116
this.logLevel = logLevel;
@@ -236,6 +238,11 @@ public Builder doNotCloseAfterDecode() {
236238
return this;
237239
}
238240

241+
public Builder exceptionPropagationPolicy(ExceptionPropagationPolicy propagationPolicy) {
242+
this.propagationPolicy = propagationPolicy;
243+
return this;
244+
}
245+
239246
public <T> T target(Class<T> apiType, String url) {
240247
return target(new HardCodedTarget<T>(apiType, url));
241248
}
@@ -247,7 +254,7 @@ public <T> T target(Target<T> target) {
247254
public Feign build() {
248255
SynchronousMethodHandler.Factory synchronousMethodHandlerFactory =
249256
new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors, logger,
250-
logLevel, decode404, closeAfterDecode);
257+
logLevel, decode404, closeAfterDecode, propagationPolicy);
251258
ParseHandlersByName handlersByName =
252259
new ParseHandlersByName(contract, options, encoder, decoder, queryMapEncoder,
253260
errorDecoder, synchronousMethodHandlerFactory);

‎core/src/main/java/feign/ReflectiveFeign.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
9999
} else if ("toString".equals(method.getName())) {
100100
return toString();
101101
}
102+
102103
return dispatch.get(method).invoke(args);
103104
}
104105

‎core/src/main/java/feign/SynchronousMethodHandler.java‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import feign.codec.DecodeException;
2222
import feign.codec.Decoder;
2323
import feign.codec.ErrorDecoder;
24+
import static feign.ExceptionPropagationPolicy.UNWRAP;
2425
import static feign.FeignException.errorExecuting;
2526
import static feign.FeignException.errorReading;
2627
import static feign.Util.checkNotNull;
@@ -43,13 +44,14 @@ final class SynchronousMethodHandler implements MethodHandler {
4344
private final ErrorDecoder errorDecoder;
4445
private final boolean decode404;
4546
private final boolean closeAfterDecode;
47+
private final ExceptionPropagationPolicy propagationPolicy;
4648

4749
private SynchronousMethodHandler(Target<?> target, Client client, Retryer retryer,
4850
List<RequestInterceptor> requestInterceptors, Logger logger,
4951
Logger.Level logLevel, MethodMetadata metadata,
5052
RequestTemplate.Factory buildTemplateFromArgs, Options options,
5153
Decoder decoder, ErrorDecoder errorDecoder, boolean decode404,
52-
boolean closeAfterDecode) {
54+
boolean closeAfterDecode, ExceptionPropagationPolicy propagationPolicy) {
5355
this.target = checkNotNull(target, "target");
5456
this.client = checkNotNull(client, "client for %s", target);
5557
this.retryer = checkNotNull(retryer, "retryer for %s", target);
@@ -64,6 +66,7 @@ private SynchronousMethodHandler(Target<?> target, Client client, Retryer retrye
6466
this.decoder = checkNotNull(decoder, "decoder for %s", target);
6567
this.decode404 = decode404;
6668
this.closeAfterDecode = closeAfterDecode;
69+
this.propagationPolicy = propagationPolicy;
6770
}
6871

6972
@Override
@@ -74,7 +77,16 @@ public Object invoke(Object[] argv) throws Throwable {
7477
try {
7578
return executeAndDecode(template);
7679
} catch (RetryableException e) {
77-
retryer.continueOrPropagate(e);
80+
try {
81+
retryer.continueOrPropagate(e);
82+
} catch (RetryableException th) {
83+
Throwable cause = th.getCause();
84+
if (propagationPolicy == UNWRAP && cause != null) {
85+
throw cause;
86+
} else {
87+
throw th;
88+
}
89+
}
7890
if (logLevel != Logger.Level.NONE) {
7991
logger.logRetry(metadata.configKey(), logLevel);
8092
}
@@ -178,16 +190,19 @@ static class Factory {
178190
private final Logger.Level logLevel;
179191
private final boolean decode404;
180192
private final boolean closeAfterDecode;
193+
private final ExceptionPropagationPolicy propagationPolicy;
181194

182195
Factory(Client client, Retryer retryer, List<RequestInterceptor> requestInterceptors,
183-
Logger logger, Logger.Level logLevel, boolean decode404, boolean closeAfterDecode) {
196+
Logger logger, Logger.Level logLevel, boolean decode404, boolean closeAfterDecode,
197+
ExceptionPropagationPolicy propagationPolicy) {
184198
this.client = checkNotNull(client, "client");
185199
this.retryer = checkNotNull(retryer, "retryer");
186200
this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors");
187201
this.logger = checkNotNull(logger, "logger");
188202
this.logLevel = checkNotNull(logLevel, "logLevel");
189203
this.decode404 = decode404;
190204
this.closeAfterDecode = closeAfterDecode;
205+
this.propagationPolicy = propagationPolicy;
191206
}
192207

193208
public MethodHandler create(Target<?> target,
@@ -198,7 +213,7 @@ public MethodHandler create(Target<?> target,
198213
ErrorDecoder errorDecoder) {
199214
return new SynchronousMethodHandler(target, client, retryer, requestInterceptors, logger,
200215
logLevel, md, buildTemplateFromArgs, options, decoder,
201-
errorDecoder, decode404, closeAfterDecode);
216+
errorDecoder, decode404, closeAfterDecode, propagationPolicy);
202217
}
203218
}
204219
}

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

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import feign.Feign.ResponseMappingDecoder;
1919
import feign.Request.HttpMethod;
2020
import feign.Target.HardCodedTarget;
21-
import feign.codec.*;
2221
import feign.querymap.BeanQueryMapEncoder;
2322
import okhttp3.mockwebserver.MockResponse;
2423
import okhttp3.mockwebserver.MockWebServer;
@@ -32,6 +31,13 @@
3231
import java.net.URI;
3332
import java.util.*;
3433
import java.util.concurrent.atomic.AtomicReference;
34+
import feign.codec.DecodeException;
35+
import feign.codec.Decoder;
36+
import feign.codec.EncodeException;
37+
import feign.codec.Encoder;
38+
import feign.codec.ErrorDecoder;
39+
import feign.codec.StringDecoder;
40+
import static feign.ExceptionPropagationPolicy.UNWRAP;
3541
import static feign.Util.UTF_8;
3642
import static feign.assertj.MockWebServerAssertions.assertThat;
3743
import static org.assertj.core.data.MapEntry.entry;
@@ -521,7 +527,7 @@ public void throwsFeignExceptionIncludingBody() {
521527
}
522528

523529
@Test
524-
public void ensureRetryerClonesItself() {
530+
public void ensureRetryerClonesItself() throws Exception {
525531
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 1"));
526532
server.enqueue(new MockResponse().setResponseCode(200).setBody("foo 2"));
527533
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 3"));
@@ -543,6 +549,51 @@ public Exception decode(String methodKey, Response response) {
543549
assertEquals(4, server.getRequestCount());
544550
}
545551

552+
@Test
553+
public void throwsOriginalExceptionAfterFailedRetries() throws Exception {
554+
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 1"));
555+
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 2"));
556+
557+
final String message = "the innerest";
558+
thrown.expect(TestInterfaceException.class);
559+
thrown.expectMessage(message);
560+
561+
TestInterface api = Feign.builder()
562+
.exceptionPropagationPolicy(UNWRAP)
563+
.retryer(new Retryer.Default(1, 1, 2))
564+
.errorDecoder(new ErrorDecoder() {
565+
@Override
566+
public Exception decode(String methodKey, Response response) {
567+
return new RetryableException("play it again sam!", HttpMethod.POST,
568+
new TestInterfaceException(message), null);
569+
}
570+
}).target(TestInterface.class, "http://localhost:" + server.getPort());
571+
572+
api.post();
573+
}
574+
575+
@Test
576+
public void throwsRetryableExceptionIfNoUnderlyingCause() throws Exception {
577+
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 1"));
578+
server.enqueue(new MockResponse().setResponseCode(503).setBody("foo 2"));
579+
580+
String message = "play it again sam!";
581+
thrown.expect(RetryableException.class);
582+
thrown.expectMessage(message);
583+
584+
TestInterface api = Feign.builder()
585+
.exceptionPropagationPolicy(UNWRAP)
586+
.retryer(new Retryer.Default(1, 1, 2))
587+
.errorDecoder(new ErrorDecoder() {
588+
@Override
589+
public Exception decode(String methodKey, Response response) {
590+
return new RetryableException(message, HttpMethod.POST, null);
591+
}
592+
}).target(TestInterface.class, "http://localhost:" + server.getPort());
593+
594+
api.post();
595+
}
596+
546597
@Test
547598
public void whenReturnTypeIsResponseNoErrorHandling() {
548599
Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
@@ -755,7 +806,7 @@ private Response responseWithText(String text) {
755806
}
756807

757808
@Test
758-
public void mapAndDecodeExecutesMapFunction() {
809+
public void mapAndDecodeExecutesMapFunction() throws Exception {
759810
server.enqueue(new MockResponse().setBody("response!"));
760811

761812
TestInterface api = new Feign.Builder()
@@ -815,7 +866,7 @@ interface TestInterface {
815866
Response response();
816867

817868
@RequestLine("POST /")
818-
String post();
869+
String post() throws TestInterfaceException;
819870

820871
@RequestLine("POST /")
821872
@Body("%7B\"customer_name\": \"{customer_name}\", \"user_name\": \"{user_name}\", \"password\": \"{password}\"%7D")
@@ -894,6 +945,11 @@ public String expand(Object value) {
894945
}
895946
}
896947

948+
class TestInterfaceException extends Exception {
949+
TestInterfaceException(String message) {
950+
super(message);
951+
}
952+
}
897953

898954
interface OtherTestInterface {
899955

0 commit comments

Comments
 (0)