Skip to content

Commit cb5fe28

Browse files
schiemonschiemon
committed
feat: add RetryConfig.noRetry() and add tests
1 parent bc52c44 commit cb5fe28

File tree

4 files changed

+95
-43
lines changed

4 files changed

+95
-43
lines changed

‎core/src/main/java/com/linecorp/armeria/client/retry/RetryConfig.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
2-
* Copyright 2020 LINE Corporation
2+
* Copyright 2025 LY Corporation
33
*
4-
* LINE Corporation licenses this file to you under the Apache License,
4+
* LY Corporation licenses this file to you under the Apache License,
55
* version 2.0 (the "License"); you may not use this file except in compliance
66
* with the License. You may obtain a copy of the License at:
77
*
@@ -26,6 +26,7 @@
2626
import com.linecorp.armeria.common.Response;
2727
import com.linecorp.armeria.common.RpcResponse;
2828
import com.linecorp.armeria.common.annotation.Nullable;
29+
import com.linecorp.armeria.common.util.UnmodifiableFuture;
2930

3031
/**
3132
* Holds retry config used by a {@link RetryingClient}.
@@ -36,6 +37,14 @@ public final class RetryConfig<T extends Response> {
3637

3738
private static final Logger logger = LoggerFactory.getLogger(RetryConfig.class);
3839

40+
private static final RetryConfig<?> NO_RETRY_CONFIG = builder(
41+
(ctx, cause) -> UnmodifiableFuture.completedFuture(RetryDecision.noRetry())).build();
42+
43+
@SuppressWarnings("unchecked")
44+
public static <T extends Response> RetryConfig<T> noRetry() {
45+
return (RetryConfig<T>) NO_RETRY_CONFIG;
46+
}
47+
3948
/**
4049
* Returns a new {@link RetryConfigBuilder} with the specified {@link RetryRule}.
4150
*/

‎core/src/test/java/com/linecorp/armeria/client/circuitbreaker/RetryConfigMappingTest.java

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ class RetryConfigMappingTest {
4343
@Test
4444
void createsCorrectMappingWithPerMethod() {
4545
final RetryConfig configForGet = RetryConfig.builder(RetryRule.failsafe()).build();
46-
final RetryConfig configForPost =
47-
RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
46+
final RetryConfig configForPost = RetryConfig.noRetry();
4847

4948
final RetryConfigMapping<?> mapping = RetryConfigMapping.perMethod(method -> {
5049
switch (method) {
@@ -98,8 +97,7 @@ void createsCorrectMappingWithPerMethod() {
9897
@Test
9998
void createsCorrectMappingWithPerRpcMethod() {
10099
final RetryConfig configForQuery = RetryConfig.builder(RetryRule.failsafe()).build();
101-
final RetryConfig configForMutation =
102-
RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
100+
final RetryConfig configForMutation = RetryConfig.noRetry();
103101

104102
final RetryConfigMapping<?> mapping = RetryConfigMapping.perMethod(method -> {
105103
switch (method) {
@@ -154,8 +152,7 @@ void createsCorrectMappingWithPerRpcMethod() {
154152
@Test
155153
void createsCorrectMappingWithPerHost() {
156154
final RetryConfig configForExampleHost = RetryConfig.builder(RetryRule.failsafe()).build();
157-
final RetryConfig configForLocalHost =
158-
RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
155+
final RetryConfig configForLocalHost = RetryConfig.noRetry();
159156

160157
final RetryConfigMapping<?> mapping = RetryConfigMapping.perHost(host -> {
161158
switch (host) {
@@ -209,8 +206,7 @@ void createsCorrectMappingWithPerHost() {
209206
@Test
210207
void createsCorrectMappingWithPerHostAndRpcRequest() {
211208
final RetryConfig configForProdHost = RetryConfig.builder(RetryRule.failsafe()).build();
212-
final RetryConfig configForTestHost =
213-
RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
209+
final RetryConfig configForTestHost = RetryConfig.noRetry();
214210

215211
final RetryConfigMapping<?> mapping = RetryConfigMapping.perHost(host -> {
216212
switch (host) {
@@ -239,8 +235,7 @@ void createsCorrectMappingWithPerHostAndRpcRequest() {
239235
@Test
240236
void createsCorrectMappingWithPerPath() {
241237
final RetryConfig configForApiPath = RetryConfig.builder(RetryRule.failsafe()).build();
242-
final RetryConfig configForHealthPath =
243-
RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
238+
final RetryConfig configForHealthPath = RetryConfig.noRetry();
244239

245240
final RetryConfigMapping<?> mapping = RetryConfigMapping.perPath(path -> {
246241
if (path.startsWith("/api/")) {
@@ -287,13 +282,20 @@ void createsCorrectMappingWithPerPath() {
287282

288283
@Test
289284
void createsCorrectMappingWithPerHostAndMethod() {
290-
final RetryConfig configForIpAddrGet = RetryConfig.builder(RetryRule.failsafe()).build();
285+
final RetryConfig configForIpAddrGet =
286+
RetryConfig.builder(RetryRule.onException()).maxTotalAttempts(5).build();
291287
final RetryConfig configForIpAddrPost =
292-
RetryConfig.builder(RetryRule.builder().onException().thenBackoff()).build();
288+
RetryConfig.noRetry();
289+
293290

294291
// Duplicated just to see that we differentiate between the methods.
295-
final RetryConfig configForUnknownGet = RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
296-
final RetryConfig configForUnknownPost = RetryConfig.builder(RetryRule.builder().onException().thenNoRetry()).build();
292+
final RetryConfig configForUnknownGet =
293+
RetryConfig.builder(RetryRule.onException()).maxTotalAttempts(10).build();
294+
// Not using RetryConfig.noRetry() here in order to have two different configs for configForIpAddrPost
295+
// and configForUnknownPost
296+
final RetryConfig configForUnknownPost = RetryConfig.builder(
297+
RetryRule.builder().onException().thenNoRetry()).build();
298+
297299

298300
final RetryConfigMapping<?> mapping = RetryConfigMapping.perHostAndMethod((host, method) -> {
299301
if ("192.168.1.1".equals(host)) {

‎core/src/test/java/com/linecorp/armeria/client/retry/RetryingClientTest.java

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
2-
* Copyright 2017 LINE Corporation
2+
* Copyright 2025 LY Corporation
33
*
4-
* LINE Corporation licenses this file to you under the Apache License,
4+
* LY Corporation licenses this file to you under the Apache License,
55
* version 2.0 (the "License"); you may not use this file except in compliance
66
* with the License. You may obtain a copy of the License at:
77
*
@@ -663,6 +663,21 @@ void shouldGetExceptionWhenFactoryIsClosed() {
663663
"(?i).*(factory has been closed|not accepting a task).*"));
664664
}
665665

666+
@Test
667+
void doNotRetryWhenNoRetryConfigIsGiven() throws InterruptedException {
668+
// test with /500-then-success. check response code and retry count
669+
final WebClient client = client(RetryConfig.noRetry(), 10000);
670+
final AggregatedHttpResponse res = client.get("/retry-after-with-http-date").aggregate().join();
671+
672+
assertThat(res.status()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE);
673+
assertThat(reqCount.get()).isEqualTo(1);
674+
675+
// Sleep 1 second more to check if there was another retry.
676+
Thread.sleep(1000);
677+
678+
assertThat(reqCount.get()).isEqualTo(1);
679+
}
680+
666681
@Test
667682
void doNotRetryWhenResponseIsAborted() throws Exception {
668683
final List<Throwable> abortCauses =
@@ -852,36 +867,36 @@ private WebClient client(RetryConfigMapping<HttpResponse> mapping) {
852867

853868
private WebClient client(RetryRule retryRule, long responseTimeoutMillis,
854869
long responseTimeoutForEach, int maxTotalAttempts) {
855-
final Function<? super HttpClient, RetryingClient> retryingDecorator =
856-
RetryingClient.builder(
857-
RetryConfig.<HttpResponse>builder0(retryRule)
858-
.responseTimeoutMillisForEachAttempt(responseTimeoutForEach)
859-
.maxTotalAttempts(maxTotalAttempts)
860-
.build())
861-
.useRetryAfter(true)
862-
.newDecorator();
863-
864-
return WebClient.builder(server.httpUri())
865-
.factory(clientFactory)
866-
.responseTimeoutMillis(responseTimeoutMillis)
867-
.decorator(retryingDecorator)
868-
.build();
870+
return client(
871+
RetryConfig.builder(retryRule)
872+
.responseTimeoutMillisForEachAttempt(responseTimeoutForEach)
873+
.maxTotalAttempts(maxTotalAttempts)
874+
.build()
875+
, responseTimeoutMillis
876+
);
869877
}
870878

871879
private WebClient client(RetryRuleWithContent<HttpResponse> retryRuleWithContent,
872880
long responseTimeoutMillis,
873881
long responseTimeoutForEach, int maxTotalAttempts) {
882+
return client(
883+
RetryConfig.builder(retryRuleWithContent)
884+
.responseTimeoutMillisForEachAttempt(responseTimeoutForEach)
885+
.maxTotalAttempts(maxTotalAttempts)
886+
.build()
887+
, responseTimeoutMillis
888+
);
889+
}
890+
891+
private WebClient client(RetryConfig<HttpResponse> retryConfig, long responseTimeoutMillis) {
874892
final Function<? super HttpClient, RetryingClient> retryingDecorator =
875-
RetryingClient.builder(retryRuleWithContent)
876-
.responseTimeoutMillisForEachAttempt(responseTimeoutForEach)
893+
RetryingClient.builder(retryConfig)
877894
.useRetryAfter(true)
878-
.maxTotalAttempts(maxTotalAttempts)
879895
.newDecorator();
880896

881897
return WebClient.builder(server.httpUri())
882898
.factory(clientFactory)
883899
.responseTimeoutMillis(responseTimeoutMillis)
884-
.decorator(LoggingClient.newDecorator())
885900
.decorator(retryingDecorator)
886901
.build();
887902
}

‎thrift/thrift0.13/src/test/java/com/linecorp/armeria/it/client/retry/RetryingRpcClientTest.java

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
2-
* Copyright 2017 LINE Corporation
2+
* Copyright 2025 LY Corporation
33
*
4-
* LINE Corporation licenses this file to you under the Apache License,
4+
* LY Corporation licenses this file to you under the Apache License,
55
* version 2.0 (the "License"); you may not use this file except in compliance
66
* with the License. You may obtain a copy of the License at:
77
*
@@ -239,16 +239,19 @@ private HelloService.Iface helloClient(RetryConfigMapping<RpcResponse> mapping)
239239
}
240240

241241
private HelloService.Iface helloClient(RetryRuleWithContent<RpcResponse> rule, int maxAttempts) {
242+
return helloClient(RetryConfig.builderForRpc(rule)
243+
.maxTotalAttempts(maxAttempts)
244+
.build());
245+
}
246+
247+
private HelloService.Iface helloClient(RetryConfig<RpcResponse> retryConfig) {
242248
return ThriftClients.builder(server.httpUri())
243249
.path("/thrift")
244-
.rpcDecorator(
245-
RetryingRpcClient.builder(RetryConfig.builderForRpc(rule)
246-
.maxTotalAttempts(maxAttempts)
247-
.build())
248-
.newDecorator())
250+
.rpcDecorator(RetryingRpcClient.newDecorator(retryConfig))
249251
.build(HelloService.Iface.class);
250252
}
251253

254+
252255
private HelloService.Iface helloClient(RetryRuleWithContent<RpcResponse> rule, int maxAttempts,
253256
BlockingQueue<RequestLog> logQueue) {
254257
return ThriftClients.builder(server.httpUri())
@@ -326,6 +329,29 @@ void shouldGetExceptionWhenFactoryIsClosed() throws Exception {
326329
"(?i).*(factory has been closed|not accepting a task).*"));
327330
}
328331

332+
@Test
333+
void doNotRetryWhenNoRetryConfigIsGiven() throws Exception {
334+
final HelloService.Iface client = helloClient(RetryConfig.noRetry());
335+
336+
when(serviceHandler.hello(anyString()))
337+
.thenThrow(new IllegalArgumentException())
338+
.thenReturn("world");
339+
340+
341+
final Throwable t = catchThrowable(() -> client.hello("hello"));
342+
343+
assertThat(t).isInstanceOf(TApplicationException.class);
344+
assertThat(((TApplicationException) t).getType()).isEqualTo(TApplicationException.INTERNAL_ERROR);
345+
346+
// Verify that the service was called only once.
347+
verify(serviceHandler, only()).hello("hello");
348+
349+
// Sleep for 1 second to check if there was another retry.
350+
TimeUnit.SECONDS.sleep(1);
351+
352+
verify(serviceHandler, only()).hello("hello");
353+
}
354+
329355
@Test
330356
void doNotRetryWhenResponseIsCancelled() throws Exception {
331357
final AtomicReference<ClientRequestContext> context = new AtomicReference<>();

0 commit comments

Comments
 (0)