Skip to content

Commit 05c5f92

Browse files
JonathanOadriancole
JonathanO
authored and
adriancole
committed
Allows configuration of status codes that cause Ribbon retries (OpenFeign#492)
1 parent 82b57a3 commit 05c5f92

4 files changed

Lines changed: 65 additions & 5 deletions

File tree

‎ribbon/src/main/java/feign/ribbon/LBClient.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import com.netflix.client.ClientRequest;
2121
import com.netflix.client.IResponse;
2222
import com.netflix.client.RequestSpecificRetryHandler;
23-
import com.netflix.client.RetryHandler;
2423
import com.netflix.client.config.CommonClientConfigKey;
2524
import com.netflix.client.config.IClientConfig;
2625
import com.netflix.loadbalancer.ILoadBalancer;
@@ -29,12 +28,14 @@
2928
import java.net.URI;
3029
import java.util.Arrays;
3130
import java.util.Collection;
31+
import java.util.Collections;
3232
import java.util.LinkedHashMap;
33+
import java.util.LinkedHashSet;
3334
import java.util.Map;
35+
import java.util.Set;
3436

3537
import feign.Client;
3638
import feign.Request;
37-
import feign.RequestTemplate;
3839
import feign.Response;
3940
import feign.Util;
4041

@@ -44,21 +45,34 @@ public final class LBClient extends
4445
private final int connectTimeout;
4546
private final int readTimeout;
4647
private final IClientConfig clientConfig;
48+
private final Set<Integer> retryableStatusCodes;
4749

4850
public static LBClient create(ILoadBalancer lb, IClientConfig clientConfig) {
4951
return new LBClient(lb, clientConfig);
5052
}
5153

54+
static Set<Integer> parseStatusCodes(String statusCodesString) {
55+
if (statusCodesString == null || statusCodesString.isEmpty()) {
56+
return Collections.emptySet();
57+
}
58+
Set<Integer> codes = new LinkedHashSet<Integer>();
59+
for (String codeString: statusCodesString.split(",")) {
60+
codes.add(Integer.parseInt(codeString));
61+
}
62+
return codes;
63+
}
64+
5265
LBClient(ILoadBalancer lb, IClientConfig clientConfig) {
5366
super(lb, clientConfig);
5467
this.clientConfig = clientConfig;
5568
connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout);
5669
readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout);
70+
retryableStatusCodes = parseStatusCodes(clientConfig.get(LBClientFactory.RetryableStatusCodes));
5771
}
5872

5973
@Override
6074
public RibbonResponse execute(RibbonRequest request, IClientConfig configOverride)
61-
throws IOException {
75+
throws IOException, ClientException {
6276
Request.Options options;
6377
if (configOverride != null) {
6478
options =
@@ -69,6 +83,10 @@ public RibbonResponse execute(RibbonRequest request, IClientConfig configOverrid
6983
options = new Request.Options(connectTimeout, readTimeout);
7084
}
7185
Response response = request.client().execute(request.toRequest(), options);
86+
if (retryableStatusCodes.contains(response.status())) {
87+
response.close();
88+
throw new ClientException(ClientException.ErrorType.SERVER_THROTTLED);
89+
}
7290
return new RibbonResponse(request.getUri(), response);
7391
}
7492

‎ribbon/src/main/java/feign/ribbon/LBClientFactory.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package feign.ribbon;
22

33
import com.netflix.client.ClientFactory;
4+
import com.netflix.client.config.CommonClientConfigKey;
45
import com.netflix.client.config.DefaultClientConfigImpl;
56
import com.netflix.client.config.IClientConfig;
7+
import com.netflix.client.config.IClientConfigKey;
68
import com.netflix.loadbalancer.ILoadBalancer;
79

810
public interface LBClientFactory {
@@ -21,10 +23,18 @@ public LBClient create(String clientName) {
2123
}
2224
}
2325

26+
IClientConfigKey<String> RetryableStatusCodes = new CommonClientConfigKey<String>("RetryableStatusCodes") {};
27+
2428
final class DisableAutoRetriesByDefaultClientConfig extends DefaultClientConfigImpl {
2529
@Override
2630
public int getDefaultMaxAutoRetriesNextServer() {
2731
return 0;
2832
}
33+
34+
@Override
35+
public void loadDefaultValues() {
36+
super.loadDefaultValues();
37+
putDefaultStringProperty(LBClientFactory.RetryableStatusCodes, "");
38+
}
2939
}
3040
}

‎ribbon/src/test/java/feign/ribbon/LBClientTest.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package feign.ribbon;
22

3-
import static org.assertj.core.api.Assertions.assertThat;
4-
53
import java.net.URI;
64
import java.net.URISyntaxException;
75
import java.nio.charset.Charset;
@@ -14,8 +12,18 @@
1412
import feign.Request;
1513
import feign.ribbon.LBClient.RibbonRequest;
1614

15+
import static org.assertj.core.api.Assertions.assertThat;
16+
1717
public class LBClientTest {
1818

19+
@Test
20+
public void testParseCodes() {
21+
assertThat(LBClient.parseStatusCodes("")).isEmpty();
22+
assertThat(LBClient.parseStatusCodes(null)).isEmpty();
23+
assertThat(LBClient.parseStatusCodes("504")).contains(504);
24+
assertThat(LBClient.parseStatusCodes("503,504")).contains(503, 504);
25+
}
26+
1927
@Test
2028
public void testRibbonRequest() throws URISyntaxException {
2129
// test for RibbonRequest.toRequest()

‎ribbon/src/test/java/feign/ribbon/RibbonClientTest.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,30 @@ public void ioExceptionRetryWithBuilder() throws IOException, InterruptedExcepti
255255
// TODO: verify ribbon stats match
256256
// assertEquals(target.lb().getLoadBalancerStats().getSingleServerStat())
257257
}
258+
259+
@Test
260+
public void ribbonRetryOnStatusCodes() throws IOException, InterruptedException {
261+
server1.enqueue(new MockResponse().setResponseCode(502));
262+
server2.enqueue(new MockResponse().setResponseCode(503));
263+
264+
getConfigInstance().setProperty(serverListKey(), hostAndPort(server1.url("").url()) + "," + hostAndPort(server2.url("").url()));
265+
getConfigInstance().setProperty(client() + ".ribbon.MaxAutoRetriesNextServer", 1);
266+
getConfigInstance().setProperty(client() + ".ribbon.RetryableStatusCodes", "503,502");
267+
268+
TestInterface
269+
api =
270+
Feign.builder().client(RibbonClient.create()).retryer(Retryer.NEVER_RETRY)
271+
.target(TestInterface.class, "http://" + client());
272+
273+
try {
274+
api.post();
275+
fail("No exception thrown");
276+
} catch (Exception ignored) {
277+
278+
}
279+
assertEquals(1, server1.getRequestCount());
280+
assertEquals(1, server2.getRequestCount());
281+
}
258282

259283
@Test
260284
public void testFeignOptionsClientConfig() {

0 commit comments

Comments
 (0)