Skip to content

Commit d5d3dfd

Browse files
newtorkMatKuhr
newtork
and
MatKuhr
authored
chore: Enhance HttpClient5 Retry-Strategy for 429 and 503 responses (#1009)
Co-authored-by: Matthias Kuhr <52661546+MatKuhr@users.noreply.github.com>
1 parent 432c829 commit d5d3dfd

3 files changed

Lines changed: 200 additions & 1 deletion

File tree

‎cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Factory.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.apache.hc.client5.http.classic.HttpClient;
1515
import org.apache.hc.client5.http.config.ConnectionConfig;
1616
import org.apache.hc.client5.http.config.RequestConfig;
17+
import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
1718
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
1819
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
1920
import org.apache.hc.client5.http.impl.classic.HttpClients;
@@ -24,8 +25,12 @@
2425
import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
2526
import org.apache.hc.client5.http.ssl.TlsSocketStrategy;
2627
import org.apache.hc.core5.http.HttpHost;
28+
import org.apache.hc.core5.http.HttpRequest;
2729
import org.apache.hc.core5.http.HttpRequestInterceptor;
30+
import org.apache.hc.core5.http.HttpResponse;
2831
import org.apache.hc.core5.http.io.SocketConfig;
32+
import org.apache.hc.core5.http.protocol.HttpContext;
33+
import org.apache.hc.core5.util.TimeValue;
2934
import org.apache.hc.core5.util.Timeout;
3035

3136
import com.sap.cloud.sdk.cloudplatform.connectivity.exception.DestinationAccessException;
@@ -92,6 +97,7 @@ private CloseableHttpClient buildHttpClient(
9297
.custom()
9398
.setConnectionManager(getConnectionManager(destination))
9499
.setDefaultRequestConfig(requestConfig)
100+
.setRetryStrategy(new LoggingHttpRequestRetryStrategy(destination))
95101
.setProxy(getProxy(destination));
96102

97103
if( requestInterceptor != null ) {
@@ -233,4 +239,54 @@ private boolean isValidProxyConfigurationUriInDestination( @Nullable final HttpD
233239
}
234240
return true;
235241
}
242+
243+
private static class LoggingHttpRequestRetryStrategy extends DefaultHttpRequestRetryStrategy
244+
{
245+
private static final int MAX_RETRIES = 1; // default
246+
private static final Duration RETRY_INTERVAL = Duration.ofSeconds(1L); // default
247+
248+
private final @Nonnull String destinationRef;
249+
250+
public LoggingHttpRequestRetryStrategy( final @Nullable HttpDestinationProperties destination )
251+
{
252+
super(MAX_RETRIES, TimeValue.of(RETRY_INTERVAL));
253+
this.destinationRef = destination == null ? "" : " for destination " + destination;
254+
}
255+
256+
@Override
257+
public boolean retryRequest( final HttpResponse response, final int execCount, final HttpContext context )
258+
{
259+
final boolean retry = super.retryRequest(response, execCount, context);
260+
if( retry ) {
261+
final String msg = "Retrying request{} due to response {}. Retry attempt {}/{} after {}s.";
262+
log.warn(msg, destinationRef, response.getCode(), execCount, MAX_RETRIES, RETRY_INTERVAL.getSeconds());
263+
}
264+
return retry;
265+
}
266+
267+
@Override
268+
public boolean retryRequest(
269+
final HttpRequest req,
270+
final IOException exception,
271+
final int execCount,
272+
final HttpContext context )
273+
{
274+
final boolean retry = super.retryRequest(req, exception, execCount, context);
275+
if( retry ) {
276+
final String msg =
277+
"Retrying {} request{} to {} due to exception \"{}\". Retry attempt {}/{} after {}s.";
278+
log
279+
.warn(
280+
msg,
281+
req.getMethod(),
282+
destinationRef,
283+
req.getRequestUri(),
284+
exception.getMessage(),
285+
execCount,
286+
MAX_RETRIES,
287+
RETRY_INTERVAL.getSeconds());
288+
}
289+
return retry;
290+
}
291+
}
236292
}

‎cloudplatform/connectivity-apache-httpclient5/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/LoggingHttpRequestRetryStrategyTest.java‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package com.sap.cloud.sdk.cloudplatform.connectivity;
2+
3+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
4+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
5+
import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
6+
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
7+
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
8+
import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED;
9+
import static org.assertj.core.api.Assertions.assertThat;
10+
11+
import java.io.IOException;
12+
13+
import org.apache.hc.client5.http.classic.methods.HttpGet;
14+
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
15+
import org.apache.hc.core5.http.HttpResponse;
16+
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.api.extension.RegisterExtension;
18+
19+
import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
20+
21+
class LoggingHttpRequestRetryStrategyTest
22+
{
23+
@RegisterExtension
24+
static final WireMockExtension WIRE_MOCK_SERVER =
25+
WireMockExtension.newInstance().options(wireMockConfig().dynamicPort()).build();
26+
27+
@Test
28+
void testHttpClientRetriesOn503()
29+
throws IOException
30+
{
31+
// Use WireMock Scenarios to simulate: first request returns 503, second returns 200
32+
WIRE_MOCK_SERVER
33+
.stubFor(
34+
get(urlEqualTo("/retry-test"))
35+
.inScenario("Retry 503")
36+
.whenScenarioStateIs(STARTED)
37+
.willReturn(aResponse().withStatus(503))
38+
.willSetStateTo("Recovered"));
39+
40+
WIRE_MOCK_SERVER
41+
.stubFor(
42+
get(urlEqualTo("/retry-test"))
43+
.inScenario("Retry 503")
44+
.whenScenarioStateIs("Recovered")
45+
.willReturn(aResponse().withStatus(200).withBody("OK")));
46+
47+
final ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder().build();
48+
49+
try( CloseableHttpClient client = (CloseableHttpClient) factory.createHttpClient() ) {
50+
final HttpGet request = new HttpGet(WIRE_MOCK_SERVER.url("/retry-test"));
51+
final int statusCode = client.execute(request, HttpResponse::getCode);
52+
53+
assertThat(statusCode).isEqualTo(200);
54+
WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/retry-test")));
55+
}
56+
}
57+
58+
@Test
59+
void testHttpClientRetriesOn429()
60+
throws IOException
61+
{
62+
// Use WireMock Scenarios to simulate: first request returns 429, second returns 200
63+
WIRE_MOCK_SERVER
64+
.stubFor(
65+
get(urlEqualTo("/rate-limit-test"))
66+
.inScenario("Retry 429")
67+
.whenScenarioStateIs(STARTED)
68+
.willReturn(aResponse().withStatus(429))
69+
.willSetStateTo("Recovered"));
70+
71+
WIRE_MOCK_SERVER
72+
.stubFor(
73+
get(urlEqualTo("/rate-limit-test"))
74+
.inScenario("Retry 429")
75+
.whenScenarioStateIs("Recovered")
76+
.willReturn(aResponse().withStatus(200).withBody("OK")));
77+
78+
final ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder().build();
79+
80+
try( CloseableHttpClient client = (CloseableHttpClient) factory.createHttpClient() ) {
81+
final HttpGet request = new HttpGet(WIRE_MOCK_SERVER.url("/rate-limit-test"));
82+
final int statusCode = client.execute(request, HttpResponse::getCode);
83+
84+
assertThat(statusCode).isEqualTo(200);
85+
WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/rate-limit-test")));
86+
}
87+
}
88+
89+
@Test
90+
void testHttpClientDoesNotRetryOn500()
91+
throws IOException
92+
{
93+
// 500 Internal Server Error should not trigger retry
94+
WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/no-retry-test")).willReturn(aResponse().withStatus(500)));
95+
96+
final ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder().build();
97+
98+
try( CloseableHttpClient client = (CloseableHttpClient) factory.createHttpClient() ) {
99+
final HttpGet request = new HttpGet(WIRE_MOCK_SERVER.url("/no-retry-test"));
100+
final int statusCode = client.execute(request, HttpResponse::getCode);
101+
102+
assertThat(statusCode).isEqualTo(500);
103+
WIRE_MOCK_SERVER.verify(1, getRequestedFor(urlEqualTo("/no-retry-test")));
104+
}
105+
}
106+
107+
@Test
108+
void testHttpClientDoesNotRetryOn400()
109+
throws IOException
110+
{
111+
// 400 Bad Request should not trigger retry
112+
WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/bad-request-test")).willReturn(aResponse().withStatus(400)));
113+
114+
final ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder().build();
115+
116+
try( CloseableHttpClient client = (CloseableHttpClient) factory.createHttpClient() ) {
117+
final HttpGet request = new HttpGet(WIRE_MOCK_SERVER.url("/bad-request-test"));
118+
final int statusCode = client.execute(request, HttpResponse::getCode);
119+
120+
assertThat(statusCode).isEqualTo(400);
121+
WIRE_MOCK_SERVER.verify(1, getRequestedFor(urlEqualTo("/bad-request-test")));
122+
}
123+
}
124+
125+
@Test
126+
void testHttpClientStopsRetryingAfterMaxAttempts()
127+
throws IOException
128+
{
129+
// Server always returns 503 - should stop after max retries (1 retry = 2 total requests)
130+
WIRE_MOCK_SERVER.stubFor(get(urlEqualTo("/always-503")).willReturn(aResponse().withStatus(503)));
131+
132+
final ApacheHttpClient5Factory factory = new ApacheHttpClient5FactoryBuilder().build();
133+
134+
try( CloseableHttpClient client = (CloseableHttpClient) factory.createHttpClient() ) {
135+
final HttpGet request = new HttpGet(WIRE_MOCK_SERVER.url("/always-503"));
136+
final int statusCode = client.execute(request, HttpResponse::getCode);
137+
138+
// DefaultHttpRequestRetryStrategy allows 1 retry, so 2 total requests
139+
assertThat(statusCode).isEqualTo(503);
140+
WIRE_MOCK_SERVER.verify(2, getRequestedFor(urlEqualTo("/always-503")));
141+
}
142+
}
143+
}

‎release_notes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
### 📈 Improvements
1818

19-
-
19+
- Enable Retry behavior for HttpClient5 instances, to mirror legacy behvior for HttpClient4: `1` retry with `1s` delay for response codes `429` (Too Many Requests) and `503` (Service Unavailable).
2020

2121
### 🐛 Fixed Issues
2222

0 commit comments

Comments
 (0)