From 8cd0291cb406b439a152c8d4aa8e976ae09becb5 Mon Sep 17 00:00:00 2001 From: Gonzalo Donaire Date: Fri, 18 Sep 2026 19:12:43 -0300 Subject: [PATCH] Apply per-call maxRetries on top of the client's retry policy RequestOptions.Builder.maxRetries(n) documented itself as "the client's policy with a different retry count", but it replaced the policy with RetryPolicy.DEFAULT.withMaxRetries(n). A client configured with custom retryable statuses or backoff silently fell back to the defaults for that call, e.g. retrying a 500 with 500 ms backoff when the client only retries 503 with 1 ms backoff. RequestOptions now carries the retry count separately and applies it to the call's retryPolicy when set, otherwise to the client's. This matches the JavaScript SDK, where partial per-call retry overrides inherit unset fields from the client. The previous three-argument constructor is kept. --- .../premocloud/typesafe/RequestOptions.java | 37 ++++++++++++++++--- .../premocloud/typesafe/TypeSafeClient.java | 2 +- .../typesafe/TypeSafeClientTest.java | 26 +++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/RequestOptions.java b/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/RequestOptions.java index 4644af1..53b2184 100644 --- a/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/RequestOptions.java +++ b/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/RequestOptions.java @@ -5,6 +5,7 @@ import java.time.Duration; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Objects; import java.util.function.Consumer; /** @@ -17,25 +18,44 @@ * * @param timeout per-attempt timeout, or {@code null} for the client's * @param retryPolicy retry policy, or {@code null} for the client's + * @param maxRetries retry count applied on top of {@code retryPolicy}, or of the client's policy when that is + * {@code null}; {@code null} keeps the policy's own count * @param headers headers added to this call; same-named client headers are replaced */ -public record RequestOptions(@Nullable Duration timeout, @Nullable RetryPolicy retryPolicy, Map headers) { +public record RequestOptions(@Nullable Duration timeout, @Nullable RetryPolicy retryPolicy, @Nullable Integer maxRetries, + Map headers) { - public static final RequestOptions NONE = new RequestOptions(null, null, Map.of()); + public static final RequestOptions NONE = new RequestOptions(null, null, null, Map.of()); public RequestOptions { + if (Objects.nonNull(maxRetries) && maxRetries < 0) { + throw new IllegalArgumentException("maxRetries must be zero or more"); + } + headers = Map.copyOf(headers); } + /** Options without a retry-count override. */ + public RequestOptions(@Nullable Duration timeout, @Nullable RetryPolicy retryPolicy, Map headers) { + this(timeout, retryPolicy, null, headers); + } + public static RequestOptions of(Consumer configure) { Builder builder = new Builder(); configure.accept(builder); return builder.build(); } + /** The policy for this call: this call's policy or the client's, with this call's retry count applied. */ + RetryPolicy resolveRetryPolicy(RetryPolicy clientPolicy) { + RetryPolicy policy = Objects.requireNonNullElse(retryPolicy, clientPolicy); + return Objects.isNull(maxRetries) ? policy : policy.withMaxRetries(maxRetries); + } + public static final class Builder { private @Nullable Duration timeout; private @Nullable RetryPolicy retryPolicy; + private @Nullable Integer maxRetries; private final Map headers = new LinkedHashMap<>(); public Builder timeout(Duration timeout) { @@ -52,9 +72,16 @@ public Builder retryPolicy(RetryPolicy retryPolicy) { return this; } - /** Shorthand for the client's policy with a different retry count; {@code 0} disables retries for this call. */ + /** + * Shorthand for the client's policy with a different retry count; {@code 0} disables retries for this call. When + * {@link #retryPolicy} is also set, the count is applied to that policy instead. + */ public Builder maxRetries(int maxRetries) { - this.retryPolicy = RetryPolicy.DEFAULT.withMaxRetries(maxRetries); + if (maxRetries < 0) { + throw new IllegalArgumentException("maxRetries must be zero or more"); + } + + this.maxRetries = maxRetries; return this; } @@ -64,7 +91,7 @@ public Builder header(String name, String value) { } public RequestOptions build() { - return new RequestOptions(timeout, retryPolicy, headers); + return new RequestOptions(timeout, retryPolicy, maxRetries, headers); } } } diff --git a/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/TypeSafeClient.java b/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/TypeSafeClient.java index 8ef9dbd..79c3513 100644 --- a/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/TypeSafeClient.java +++ b/typesafe-sdk/src/main/java/io/github/premocloud/typesafe/TypeSafeClient.java @@ -152,7 +152,7 @@ private T post(String path, Object body, Class type, RequestOptions optio private T send(HttpRequest.Builder template, Class type, RequestOptions options) { Duration timeout = Objects.requireNonNullElse(options.timeout(), this.timeout); - RetryPolicy retryPolicy = Objects.requireNonNullElse(options.retryPolicy(), this.retryPolicy); + RetryPolicy retryPolicy = options.resolveRetryPolicy(this.retryPolicy); Map headers = new LinkedHashMap<>(defaultHeaders); headers.putAll(options.headers()); template.timeout(timeout) diff --git a/typesafe-sdk/src/test/java/io/github/premocloud/typesafe/TypeSafeClientTest.java b/typesafe-sdk/src/test/java/io/github/premocloud/typesafe/TypeSafeClientTest.java index 55e1c9f..a592181 100644 --- a/typesafe-sdk/src/test/java/io/github/premocloud/typesafe/TypeSafeClientTest.java +++ b/typesafe-sdk/src/test/java/io/github/premocloud/typesafe/TypeSafeClientTest.java @@ -10,6 +10,7 @@ import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.Set; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -250,6 +251,31 @@ void perCallOptionsOverrideTimeoutRetryAndHeaders() { assertThrows(IllegalArgumentException.class, () -> RequestOptions.of(o -> o.timeout(Duration.ZERO))); } + @Test + void perCallMaxRetriesKeepsTheRestOfTheClientPolicy() { + TypeSafeClient onlyRetries503 = TypeSafeClient.builder().apiKey(API_KEY).baseUrl(server.baseUrl()) + .retryPolicy(RetryPolicy.of(r -> r.maxRetries(0).httpStatuses(Set.of(503)).backoffInitial(Duration.ofMillis(1)))).build(); + + server.reply(500, "not retryable for this client"); + assertThrows(TypeSafeInternalServerException.class, + () -> onlyRetries503.systemOne(spamRequest(), RequestOptions.of(o -> o.maxRetries(2)))); + assertEquals(1, server.recorded().size()); + + server.reply(503, "busy"); + server.reply(503, "still busy"); + assertThrows(TypeSafeInternalServerException.class, + () -> onlyRetries503.systemOne(spamRequest(), RequestOptions.of(o -> o.maxRetries(1)))); + assertEquals(3, server.recorded().size()); + + RetryPolicy callPolicy = RetryPolicy.of(r -> r.maxRetries(0).httpStatuses(Set.of(500)).backoffInitial(Duration.ofMillis(1))); + server.reply(500, "one"); + server.reply(200, RESPONSE_JSON); + onlyRetries503.systemOne(spamRequest(), RequestOptions.of(o -> o.retryPolicy(callPolicy).maxRetries(1))); + assertEquals(5, server.recorded().size()); + + assertThrows(IllegalArgumentException.class, () -> RequestOptions.of(o -> o.maxRetries(-1))); + } + @Test void systemOneRejectsUnreadableBody() { server.reply(200, "not json");