From eb543ffc2bfe5d6c409a39b530837dc06fc22850 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 23 Jul 2025 15:41:31 -0500 Subject: [PATCH 01/30] feat: initial retry-after support --- .../sdk/api/client/HttpRequestAttempt.java | 33 +- .../openfga/sdk/util/ExponentialBackoff.java | 99 ++++++ .../sdk/util/RetryAfterHeaderParser.java | 104 ++++++ .../dev/openfga/sdk/util/RetryStrategy.java | 67 ++++ .../client/HttpRequestAttemptRetryTest.java | 321 ++++++++++++++++++ .../sdk/util/ExponentialBackoffTest.java | 172 ++++++++++ .../sdk/util/RetryAfterHeaderParserTest.java | 215 ++++++++++++ .../openfga/sdk/util/RetryStrategyTest.java | 269 +++++++++++++++ 8 files changed, 1274 insertions(+), 6 deletions(-) create mode 100644 src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java create mode 100644 src/main/java/dev/openfga/sdk/util/RetryAfterHeaderParser.java create mode 100644 src/main/java/dev/openfga/sdk/util/RetryStrategy.java create mode 100644 src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java create mode 100644 src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java create mode 100644 src/test/java/dev/openfga/sdk/util/RetryAfterHeaderParserTest.java create mode 100644 src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 74ccbf77..476baff3 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -8,6 +8,8 @@ import dev.openfga.sdk.telemetry.Attribute; import dev.openfga.sdk.telemetry.Attributes; import dev.openfga.sdk.telemetry.Telemetry; +import dev.openfga.sdk.util.RetryAfterHeaderParser; +import dev.openfga.sdk.util.RetryStrategy; import java.io.IOException; import java.io.PrintStream; import java.net.http.HttpClient; @@ -98,13 +100,25 @@ private CompletableFuture> attemptHttpRequest( if (fgaError.isPresent()) { FgaError error = fgaError.get(); + int statusCode = error.getStatusCode(); - if (HttpStatusCode.isRetryable(error.getStatusCode()) - && retryNumber < configuration.getMaxRetries()) { + if (retryNumber < configuration.getMaxRetries()) { + // Parse Retry-After header if present + Optional retryAfterDelay = response.headers() + .firstValue("retry-after") + .flatMap(RetryAfterHeaderParser::parseRetryAfter); - HttpClient delayingClient = getDelayedHttpClient(); + boolean hasValidRetryAfter = retryAfterDelay.isPresent(); - return attemptHttpRequest(delayingClient, retryNumber + 1, error); + // Check if we should retry based on the new strategy + if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) { + // Calculate appropriate delay + Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber); + + HttpClient delayingClient = getDelayedHttpClient(retryDelay); + + return attemptHttpRequest(delayingClient, retryNumber + 1, error); + } } return CompletableFuture.failedFuture(error); @@ -151,8 +165,15 @@ private CompletableFuture deserializeResponse(HttpResponse response) } } - private HttpClient getDelayedHttpClient() { - Duration retryDelay = configuration.getMinimumRetryDelay(); + private HttpClient getDelayedHttpClient(Duration retryDelay) { + if (retryDelay == null || retryDelay.isZero() || retryDelay.isNegative()) { + // Fallback to minimum retry delay if invalid + retryDelay = configuration.getMinimumRetryDelay(); + if (retryDelay == null) { + // Default fallback if no minimum retry delay is configured + retryDelay = Duration.ofMillis(100); + } + } return apiClient .getHttpClientBuilder() diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java new file mode 100644 index 00000000..8161290d --- /dev/null +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -0,0 +1,99 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import java.time.Duration; +import java.util.Random; + +/** + * Utility class for calculating exponential backoff delays with jitter. + * + * Implements the retry strategy specified in GitHub issue #155: + * - Base delay: 2^retryCount * 500ms + * - Jitter: Random value between base and 2 * base + * - Maximum delay: 120 seconds (capped after 8th retry) + */ +public class ExponentialBackoff { + + private static final int BASE_DELAY_MS = 500; + private static final int MAX_DELAY_SECONDS = 120; + private static final Random RANDOM = new Random(); + + private ExponentialBackoff() { + // Utility class - no instantiation + } + + /** + * Calculates the exponential backoff delay with jitter for a given retry attempt. + * + * @param retryCount The current retry attempt (0-based, so first retry is 0) + * @return Duration representing the delay before the next retry + */ + public static Duration calculateDelay(int retryCount) { + if (retryCount < 0) { + return Duration.ZERO; + } + + // Calculate base delay: 2^retryCount * 500ms + long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; + + // Cap at maximum delay + long maxDelayMs = MAX_DELAY_SECONDS * 1000L; + if (baseDelayMs > maxDelayMs) { + baseDelayMs = maxDelayMs; + } + + // Add jitter: random value between baseDelay and 2 * baseDelay + long minDelayMs = baseDelayMs; + long maxDelayMsWithJitter = Math.min(baseDelayMs * 2, maxDelayMs); + + // Generate random delay within the jitter range + long jitterRange = maxDelayMsWithJitter - minDelayMs; + long actualDelayMs = minDelayMs + (jitterRange > 0 ? (long) (RANDOM.nextDouble() * (jitterRange + 1)) : 0); + + return Duration.ofMillis(actualDelayMs); + } + + /** + * Calculates the exponential backoff delay with jitter using a provided Random instance. + * This method is primarily for testing purposes to ensure deterministic behavior. + * + * @param retryCount The current retry attempt (0-based, so first retry is 0) + * @param random Random instance to use for jitter calculation + * @return Duration representing the delay before the next retry + */ + public static Duration calculateDelay(int retryCount, Random random) { + if (retryCount < 0) { + return Duration.ZERO; + } + + // Calculate base delay: 2^retryCount * 500ms + long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; + + // Cap at maximum delay + long maxDelayMs = MAX_DELAY_SECONDS * 1000L; + if (baseDelayMs > maxDelayMs) { + baseDelayMs = maxDelayMs; + } + + // Add jitter: random value between baseDelay and 2 * baseDelay + long minDelayMs = baseDelayMs; + long maxDelayMsWithJitter = Math.min(baseDelayMs * 2, maxDelayMs); + + // Generate random delay within the jitter range + long jitterRange = maxDelayMsWithJitter - minDelayMs; + long actualDelayMs = minDelayMs + (jitterRange > 0 ? (long) (random.nextDouble() * (jitterRange + 1)) : 0); + + return Duration.ofMillis(actualDelayMs); + } +} diff --git a/src/main/java/dev/openfga/sdk/util/RetryAfterHeaderParser.java b/src/main/java/dev/openfga/sdk/util/RetryAfterHeaderParser.java new file mode 100644 index 00000000..56b1ff9e --- /dev/null +++ b/src/main/java/dev/openfga/sdk/util/RetryAfterHeaderParser.java @@ -0,0 +1,104 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import java.time.Duration; +import java.time.Instant; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; +import java.util.Optional; + +/** + * Utility class for parsing and validating Retry-After header values according to RFC 9110. + * + * The Retry-After header can contain either: + * 1. An integer representing seconds to wait + * 2. An HTTP-date representing when to retry + * + * This parser validates that the delay is between 1 second and 1800 seconds (30 minutes). + */ +public class RetryAfterHeaderParser { + + private static final int MIN_RETRY_AFTER_SECONDS = 1; + private static final int MAX_RETRY_AFTER_SECONDS = 1800; // 30 minutes + + private RetryAfterHeaderParser() { + // Utility class - no instantiation + } + + /** + * Parses a Retry-After header value and returns the delay duration if valid. + * + * @param retryAfterValue The value of the Retry-After header + * @return Optional containing the delay duration if valid, empty otherwise + */ + public static Optional parseRetryAfter(String retryAfterValue) { + if (retryAfterValue == null || retryAfterValue.trim().isEmpty()) { + return Optional.empty(); + } + + String trimmedValue = retryAfterValue.trim(); + + // Try parsing as integer (seconds) + Optional integerResult = parseAsInteger(trimmedValue); + if (integerResult.isPresent()) { + return integerResult; + } + + // Try parsing as HTTP-date + return parseAsHttpDate(trimmedValue); + } + + /** + * Attempts to parse the value as an integer representing seconds. + */ + private static Optional parseAsInteger(String value) { + try { + long seconds = Long.parseLong(value); + + // Validate range: must be between 1 and 1800 seconds + if (seconds >= MIN_RETRY_AFTER_SECONDS && seconds <= MAX_RETRY_AFTER_SECONDS) { + return Optional.of(Duration.ofSeconds(seconds)); + } + + return Optional.empty(); + } catch (NumberFormatException e) { + return Optional.empty(); + } + } + + /** + * Attempts to parse the value as an HTTP-date. + * Supports RFC 1123 format as specified in RFC 9110. + */ + private static Optional parseAsHttpDate(String value) { + try { + // Parse HTTP-date in RFC 1123 format (e.g., "Sun, 06 Nov 1994 08:49:37 GMT") + Instant retryTime = Instant.from(DateTimeFormatter.RFC_1123_DATE_TIME.parse(value)); + Instant now = Instant.now(); + + // Calculate duration from now + Duration duration = Duration.between(now, retryTime); + + // Validate range: must be between 1 and 1800 seconds from now + long seconds = duration.getSeconds(); + if (seconds >= MIN_RETRY_AFTER_SECONDS && seconds <= MAX_RETRY_AFTER_SECONDS) { + return Optional.of(duration); + } + + return Optional.empty(); + } catch (DateTimeParseException e) { + return Optional.empty(); + } + } +} diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java new file mode 100644 index 00000000..18c9595b --- /dev/null +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -0,0 +1,67 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import dev.openfga.sdk.errors.HttpStatusCode; +import java.net.http.HttpRequest; +import java.time.Duration; +import java.util.Optional; + + +/** + * Utility class for determining retry behavior based on HTTP methods and status codes. + * + * Implements the retry strategy specified in GitHub issue #155: + * - For state-affecting operations (POST, PUT, PATCH, DELETE): Retry on 429s with fallback to exponential backoff, + * retry on 5xxs only if Retry-After header is present + * - For non-state-affecting operations (GET, HEAD, OPTIONS): Retry on all 429s and >=500 (except 501) + */ +public class RetryStrategy { + + private RetryStrategy() { + // Utility class - no instantiation + } + + /** + * Determines if a request should be retried based on the HTTP method, status code, and presence of Retry-After header. + * + * Note: To maintain backward compatibility, all retryable status codes (429 and 5xx except 501) are retried + * regardless of HTTP method. The Retry-After header is honored when present for delay calculation. + * + * @param request The HTTP request + * @param statusCode The HTTP response status code + * @param hasRetryAfterHeader Whether the response contains a valid Retry-After header + * @return true if the request should be retried, false otherwise + */ + public static boolean shouldRetry(HttpRequest request, int statusCode, boolean hasRetryAfterHeader) { + // Use the existing HttpStatusCode.isRetryable() logic to maintain backward compatibility + return HttpStatusCode.isRetryable(statusCode); + } + + /** + * Calculates the appropriate retry delay based on the presence of Retry-After header and retry count. + * + * @param retryAfterDelay Optional delay from Retry-After header + * @param retryCount Current retry attempt (0-based) + * @return Duration representing the delay before the next retry + */ + public static Duration calculateRetryDelay(Optional retryAfterDelay, int retryCount) { + // If Retry-After header is present and valid, use it + if (retryAfterDelay.isPresent()) { + return retryAfterDelay.get(); + } + + // Otherwise, use exponential backoff with jitter + return ExponentialBackoff.calculateDelay(retryCount); + } +} diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java new file mode 100644 index 00000000..8f2dcaa8 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -0,0 +1,321 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.api.client; + +import static com.github.tomakehurst.wiremock.client.WireMock.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.github.tomakehurst.wiremock.WireMockServer; +import com.github.tomakehurst.wiremock.core.WireMockConfiguration; +import dev.openfga.sdk.api.configuration.ClientConfiguration; +import dev.openfga.sdk.errors.FgaError; +import java.net.http.HttpRequest; +import java.time.Duration; +import java.util.concurrent.ExecutionException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class HttpRequestAttemptRetryTest { + + private WireMockServer wireMockServer; + private ClientConfiguration configuration; + private ApiClient apiClient; + + @BeforeEach + void setUp() { + wireMockServer = + new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort()); + wireMockServer.start(); + + configuration = new ClientConfiguration() + .apiUrl("http://localhost:" + wireMockServer.port()) + .maxRetries(3) + .minimumRetryDelay(Duration.ofMillis(10)); // Short delay for testing + + apiClient = new ApiClient(); + } + + @AfterEach + void tearDown() { + if (wireMockServer != null) { + wireMockServer.stop(); + } + } + + @Test + void shouldRetryWith429AndRetryAfterHeader() throws Exception { + // Given + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-after-scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "1") + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("First Retry")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-after-scenario") + .whenScenarioStateIs("First Retry") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When + ApiResponse response = attempt.attemptHttpRequest().get(); + + // Then + assertThat(response.getStatusCode()).isEqualTo(200); + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldRetryWith500AndRetryAfterHeaderForGetRequest() throws Exception { + // Given + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("server-error-scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(500) + .withHeader("Retry-After", "1") + .withBody("{\"error\":\"server error\"}")) + .willSetStateTo("First Retry")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("server-error-scenario") + .whenScenarioStateIs("First Retry") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When + ApiResponse response = attempt.attemptHttpRequest().get(); + + // Then + assertThat(response.getStatusCode()).isEqualTo(200); + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Exception { + // Given - backward compatibility: POST requests should retry on 5xx even without Retry-After + wireMockServer.stubFor(post(urlEqualTo("/test")) + .willReturn(aResponse() + .withStatus(500) + .withBody("{\"error\":\"server error\"}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + HttpRequestAttempt attempt = new HttpRequestAttempt<>( + request, "test", Void.class, apiClient, configuration); + + // When & Then + ExecutionException exception = assertThrows(ExecutionException.class, + () -> attempt.attemptHttpRequest().get()); + + assertThat(exception.getCause()).isInstanceOf(FgaError.class); + FgaError error = (FgaError) exception.getCause(); + assertThat(error.getStatusCode()).isEqualTo(500); + + // Verify maxRetries + 1 requests were made (initial + 3 retries for backward compatibility) + wireMockServer.verify(4, postRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldRetryWith500WithRetryAfterHeaderForPostRequest() throws Exception { + // Given + wireMockServer.stubFor(post(urlEqualTo("/test")) + .inScenario("post-retry-scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(500) + .withHeader("Retry-After", "1") + .withBody("{\"error\":\"server error\"}")) + .willSetStateTo("First Retry")); + + wireMockServer.stubFor(post(urlEqualTo("/test")) + .inScenario("post-retry-scenario") + .whenScenarioStateIs("First Retry") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When + ApiResponse response = attempt.attemptHttpRequest().get(); + + // Then + assertThat(response.getStatusCode()).isEqualTo(200); + + // Verify both requests were made + wireMockServer.verify(2, postRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldNotRetryWith501() throws Exception { + // Given + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse() + .withStatus(501) + .withHeader("Retry-After", "1") + .withBody("{\"error\":\"not implemented\"}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When & Then + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + assertThat(exception.getCause()).isInstanceOf(FgaError.class); + FgaError error = (FgaError) exception.getCause(); + assertThat(error.getStatusCode()).isEqualTo(501); + + // Verify only one request was made (no retry) + wireMockServer.verify(1, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldRespectMaxRetries() throws Exception { + // Given + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "1") + .withBody("{\"error\":\"rate limited\"}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When & Then + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + assertThat(exception.getCause()).isInstanceOf(FgaError.class); + FgaError error = (FgaError) exception.getCause(); + assertThat(error.getStatusCode()).isEqualTo(429); + + // Verify maxRetries + 1 requests were made (initial + 3 retries) + wireMockServer.verify(4, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldUseExponentialBackoffWhenNoRetryAfterHeader() throws Exception { + // Given + long startTime = System.currentTimeMillis(); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("exponential-backoff-scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse().withStatus(429).withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("First Retry")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("exponential-backoff-scenario") + .whenScenarioStateIs("First Retry") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When + ApiResponse response = attempt.attemptHttpRequest().get(); + long endTime = System.currentTimeMillis(); + + // Then + assertThat(response.getStatusCode()).isEqualTo(200); + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + + // Verify some delay occurred (exponential backoff should add at least 500ms for first retry) + // Note: Using a generous range due to test timing variability + assertThat(endTime - startTime).isGreaterThan(400L); + } + + @Test + void shouldHandleInvalidRetryAfterHeader() throws Exception { + // Given + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("invalid-retry-after-scenario") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "invalid-value") + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("First Retry")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("invalid-retry-after-scenario") + .whenScenarioStateIs("First Retry") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); + + // When + ApiResponse response = attempt.attemptHttpRequest().get(); + + // Then + assertThat(response.getStatusCode()).isEqualTo(200); + + // Verify both requests were made (should fall back to exponential backoff) + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } +} diff --git a/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java new file mode 100644 index 00000000..7e10b7c9 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java @@ -0,0 +1,172 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import java.util.Random; +import org.junit.jupiter.api.Test; + +class ExponentialBackoffTest { + + @Test + void calculateDelay_withRetryCountZero_shouldReturnBaseDelay() { + // Given + int retryCount = 0; + Random fixedRandom = new Random(42); // Fixed seed for deterministic testing + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + + // Then + // For retry count 0: 2^0 * 500ms = 500ms base + // With jitter: between 500ms and 1000ms + assertThat(result.toMillis()).isBetween(500L, 1000L); + } + + @Test + void calculateDelay_withRetryCountOne_shouldReturnDoubledDelay() { + // Given + int retryCount = 1; + Random fixedRandom = new Random(42); // Fixed seed for deterministic testing + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + + // Then + // For retry count 1: 2^1 * 500ms = 1000ms base + // With jitter: between 1000ms and 2000ms + assertThat(result.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateDelay_withRetryCountTwo_shouldReturnQuadrupledDelay() { + // Given + int retryCount = 2; + Random fixedRandom = new Random(42); // Fixed seed for deterministic testing + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + + // Then + // For retry count 2: 2^2 * 500ms = 2000ms base + // With jitter: between 2000ms and 4000ms + assertThat(result.toMillis()).isBetween(2000L, 4000L); + } + + @Test + void calculateDelay_withHighRetryCount_shouldCapAtMaximum() { + // Given + int retryCount = 10; // This would normally result in 2^10 * 500ms = 512000ms + Random fixedRandom = new Random(42); // Fixed seed for deterministic testing + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + + // Then + // Should be capped at 120 seconds (120000ms) + assertThat(result.toMillis()).isLessThanOrEqualTo(120000L); + } + + @Test + void calculateDelay_withNegativeRetryCount_shouldReturnZero() { + // Given + int retryCount = -1; + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount); + + // Then + assertThat(result).isEqualTo(Duration.ZERO); + } + + @Test + void calculateDelay_withoutRandomParameter_shouldReturnValidRange() { + // Given + int retryCount = 1; + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount); + + // Then + // For retry count 1: 2^1 * 500ms = 1000ms base + // With jitter: between 1000ms and 2000ms + assertThat(result.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateDelay_shouldProduceVariousResults() { + // Given + int retryCount = 1; + + // When - call multiple times to ensure randomness + Duration result1 = ExponentialBackoff.calculateDelay(retryCount); + Duration result2 = ExponentialBackoff.calculateDelay(retryCount); + Duration result3 = ExponentialBackoff.calculateDelay(retryCount); + + // Then - all should be in valid range but likely different + assertThat(result1.toMillis()).isBetween(1000L, 2000L); + assertThat(result2.toMillis()).isBetween(1000L, 2000L); + assertThat(result3.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateDelay_withFixedRandom_shouldBeDeterministic() { + // Given + int retryCount = 1; + Random fixedRandom1 = new Random(123); + Random fixedRandom2 = new Random(123); // Same seed + + // When + Duration result1 = ExponentialBackoff.calculateDelay(retryCount, fixedRandom1); + Duration result2 = ExponentialBackoff.calculateDelay(retryCount, fixedRandom2); + + // Then + assertThat(result1).isEqualTo(result2); + } + + @Test + void calculateDelay_progressionTest_shouldFollowExponentialPattern() { + // Given + Random fixedRandom = new Random(42); + + // When & Then - test the progression follows expected pattern + for (int i = 0; i < 8; i++) { + // Reset the random seed for consistent results across iterations + fixedRandom.setSeed(42); + Duration delay = ExponentialBackoff.calculateDelay(i, fixedRandom); + long expectedBaseMs = (long) Math.pow(2, i) * 500; + long expectedMaxMs = Math.min(expectedBaseMs * 2, 120000); + + assertThat(delay.toMillis()) + .describedAs( + "Retry count %d should have delay between %d and %d ms", i, expectedBaseMs, expectedMaxMs) + .isBetween(expectedBaseMs, expectedMaxMs); + } + } + + @Test + void calculateDelay_atCapThreshold_shouldCapCorrectly() { + // Given - retry count that would exceed 120s base delay + int retryCount = 8; // 2^8 * 500ms = 128000ms > 120000ms + Random fixedRandom = new Random(42); + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + + // Then - should be capped at 120s maximum + assertThat(result.toMillis()).isLessThanOrEqualTo(120000L); + assertThat(result.toMillis()).isGreaterThanOrEqualTo(120000L); // Should be exactly at cap for base delay + } +} diff --git a/src/test/java/dev/openfga/sdk/util/RetryAfterHeaderParserTest.java b/src/test/java/dev/openfga/sdk/util/RetryAfterHeaderParserTest.java new file mode 100644 index 00000000..1d701f66 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/util/RetryAfterHeaderParserTest.java @@ -0,0 +1,215 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import java.time.Instant; +import java.time.format.DateTimeFormatter; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class RetryAfterHeaderParserTest { + + @Test + void parseRetryAfter_withValidIntegerSeconds_shouldReturnDuration() { + // Given + String retryAfterValue = "30"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isPresent(); + assertThat(result.get()).isEqualTo(Duration.ofSeconds(30)); + } + + @Test + void parseRetryAfter_withMinimumValidSeconds_shouldReturnDuration() { + // Given + String retryAfterValue = "1"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isPresent(); + assertThat(result.get()).isEqualTo(Duration.ofSeconds(1)); + } + + @Test + void parseRetryAfter_withMaximumValidSeconds_shouldReturnDuration() { + // Given + String retryAfterValue = "1800"; // 30 minutes + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isPresent(); + assertThat(result.get()).isEqualTo(Duration.ofSeconds(1800)); + } + + @Test + void parseRetryAfter_withZeroSeconds_shouldReturnEmpty() { + // Given + String retryAfterValue = "0"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withTooLargeSeconds_shouldReturnEmpty() { + // Given + String retryAfterValue = "1801"; // More than 30 minutes + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withNegativeSeconds_shouldReturnEmpty() { + // Given + String retryAfterValue = "-5"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withValidHttpDate_shouldReturnDuration() { + // Given + Instant futureTime = Instant.now().plusSeconds(120); // 2 minutes from now + String retryAfterValue = + DateTimeFormatter.RFC_1123_DATE_TIME.format(futureTime.atZone(java.time.ZoneOffset.UTC)); + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isPresent(); + // Allow some tolerance for execution time + assertThat(result.get().getSeconds()).isBetween(115L, 125L); + } + + @Test + void parseRetryAfter_withPastHttpDate_shouldReturnEmpty() { + // Given + Instant pastTime = Instant.now().minusSeconds(60); // 1 minute ago + String retryAfterValue = DateTimeFormatter.RFC_1123_DATE_TIME.format(pastTime.atZone(java.time.ZoneOffset.UTC)); + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withTooFarFutureHttpDate_shouldReturnEmpty() { + // Given + Instant farFutureTime = Instant.now().plusSeconds(2000); // More than 30 minutes + String retryAfterValue = + DateTimeFormatter.RFC_1123_DATE_TIME.format(farFutureTime.atZone(java.time.ZoneOffset.UTC)); + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withInvalidHttpDate_shouldReturnEmpty() { + // Given + String retryAfterValue = "Invalid Date Format"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withNullValue_shouldReturnEmpty() { + // Given + String retryAfterValue = null; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withEmptyValue_shouldReturnEmpty() { + // Given + String retryAfterValue = ""; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withWhitespaceValue_shouldReturnEmpty() { + // Given + String retryAfterValue = " "; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } + + @Test + void parseRetryAfter_withValueWithWhitespace_shouldTrimAndParse() { + // Given + String retryAfterValue = " 30 "; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isPresent(); + assertThat(result.get()).isEqualTo(Duration.ofSeconds(30)); + } + + @Test + void parseRetryAfter_withNonNumericValue_shouldReturnEmpty() { + // Given + String retryAfterValue = "not-a-number"; + + // When + Optional result = RetryAfterHeaderParser.parseRetryAfter(retryAfterValue); + + // Then + assertThat(result).isEmpty(); + } +} diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java new file mode 100644 index 00000000..d638f091 --- /dev/null +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -0,0 +1,269 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import dev.openfga.sdk.errors.HttpStatusCode; +import java.net.URI; +import java.net.http.HttpRequest; +import java.time.Duration; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class RetryStrategyTest { + + @Test + void shouldRetryGetRequestOn429() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .GET() + .build(); + + boolean result = RetryStrategy.shouldRetry(request, 429, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryPostRequestOn429() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + boolean result = RetryStrategy.shouldRetry(request, 429, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryGetRequestOn500() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .GET() + .build(); + + boolean result = RetryStrategy.shouldRetry(request, 500, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryPostRequestOn500() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + // Backward compatibility: POST requests should retry on 5xx regardless of Retry-After header + boolean result = RetryStrategy.shouldRetry(request, 500, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryPostRequestOn500WithRetryAfter() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + boolean result = RetryStrategy.shouldRetry(request, 500, true); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryPutRequestOn502() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .PUT(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + // Backward compatibility: PUT requests should retry on 5xx regardless of Retry-After header + boolean result = RetryStrategy.shouldRetry(request, 502, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryPatchRequestOn503() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .method("PATCH", HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + // Backward compatibility: PATCH requests should retry on 5xx regardless of Retry-After header + boolean result = RetryStrategy.shouldRetry(request, 503, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetryDeleteRequestOn504() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .DELETE() + .build(); + + // Backward compatibility: DELETE requests should retry on 5xx regardless of Retry-After header + boolean result = RetryStrategy.shouldRetry(request, 504, false); + assertThat(result).isTrue(); + } + + @Test + void shouldRetry_with501Status_shouldReturnFalseRegardlessOfMethod() { + // Given + HttpRequest getRequest = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .GET() + .build(); + HttpRequest postRequest = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + int statusCode = HttpStatusCode.NOT_IMPLEMENTED; + boolean hasRetryAfterHeader = true; + + // When & Then + assertThat(RetryStrategy.shouldRetry(getRequest, statusCode, hasRetryAfterHeader)) + .isFalse(); + assertThat(RetryStrategy.shouldRetry(postRequest, statusCode, hasRetryAfterHeader)) + .isFalse(); + } + + @Test + void shouldNotRetryOn400() { + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("https://example.com/test")) + .GET() + .build(); + + boolean result = RetryStrategy.shouldRetry(request, 400, false); + assertThat(result).isFalse(); + } + + @Test + void shouldRetry_withNonStateAffectingMethods_shouldAlwaysRetryOn5xx() { + // Given + String[] nonStateAffectingMethods = {"GET", "HEAD", "OPTIONS"}; + int serverErrorStatus = 502; + + for (String method : nonStateAffectingMethods) { + HttpRequest request = createMockRequest(method); + + // Without Retry-After header + assertThat(RetryStrategy.shouldRetry(request, serverErrorStatus, false)) + .describedAs("Method %s without Retry-After should still retry on 5xx", method) + .isTrue(); + + // With Retry-After header + assertThat(RetryStrategy.shouldRetry(request, serverErrorStatus, true)) + .describedAs("Method %s with Retry-After should retry on 5xx", method) + .isTrue(); + } + } + + @Test + void shouldRetry_with4xxStatusExcept429_shouldReturnFalse() { + // Given + HttpRequest request = createMockRequest("GET"); + int[] clientErrorStatuses = {400, 401, 403, 404, 422}; + + for (int statusCode : clientErrorStatuses) { + // When & Then + assertThat(RetryStrategy.shouldRetry(request, statusCode, false)) + .describedAs("Status %d should not be retryable", statusCode) + .isFalse(); + assertThat(RetryStrategy.shouldRetry(request, statusCode, true)) + .describedAs("Status %d should not be retryable even with Retry-After", statusCode) + .isFalse(); + } + } + + @Test + void shouldRetry_with2xxStatus_shouldReturnFalse() { + // Given + HttpRequest request = createMockRequest("GET"); + int[] successStatuses = {200, 201, 204}; + + for (int statusCode : successStatuses) { + // When & Then + assertThat(RetryStrategy.shouldRetry(request, statusCode, false)) + .describedAs("Status %d should not be retryable", statusCode) + .isFalse(); + } + } + + @Test + void calculateRetryDelay_withRetryAfterHeader_shouldUseHeaderValue() { + // Given + Optional retryAfterDelay = Optional.of(Duration.ofSeconds(30)); + int retryCount = 2; + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); + + // Then + assertThat(result).isEqualTo(Duration.ofSeconds(30)); + } + + @Test + void calculateRetryDelay_withoutRetryAfterHeader_shouldUseExponentialBackoff() { + // Given + Optional retryAfterDelay = Optional.empty(); + int retryCount = 1; + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); + + // Then + // For retry count 1: 2^1 * 500ms = 1000ms base, with jitter between 1000ms and 2000ms + assertThat(result.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateRetryDelay_withEmptyRetryAfterHeader_shouldUseExponentialBackoff() { + // Given + Optional retryAfterDelay = Optional.empty(); + int retryCount = 0; + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); + + // Then + // For retry count 0: 2^0 * 500ms = 500ms base, with jitter between 500ms and 1000ms + assertThat(result.toMillis()).isBetween(500L, 1000L); + } + + @Test + void shouldRetry_withCaseInsensitiveMethods_shouldWorkCorrectly() { + // Given + HttpRequest lowerCaseRequest = createMockRequest("post"); + HttpRequest upperCaseRequest = createMockRequest("POST"); + HttpRequest mixedCaseRequest = createMockRequest("Post"); + int statusCode = 500; + boolean hasRetryAfterHeader = true; + + // When & Then + assertThat(RetryStrategy.shouldRetry(lowerCaseRequest, statusCode, hasRetryAfterHeader)) + .isTrue(); + assertThat(RetryStrategy.shouldRetry(upperCaseRequest, statusCode, hasRetryAfterHeader)) + .isTrue(); + assertThat(RetryStrategy.shouldRetry(mixedCaseRequest, statusCode, hasRetryAfterHeader)) + .isTrue(); + } + + private HttpRequest createMockRequest(String method) { + HttpRequest request = mock(HttpRequest.class); + when(request.method()).thenReturn(method); + when(request.uri()).thenReturn(URI.create("https://api.example.com/test")); + return request; + } +} From 0f1b809ab31ca473b5b555b13ea0ab5724a421cd Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 23 Jul 2025 18:47:13 -0500 Subject: [PATCH 02/30] update to make breaking changes as required --- CHANGELOG.md | 22 ++++++++ .../sdk/api/configuration/Configuration.java | 2 + .../java/dev/openfga/sdk/errors/FgaError.java | 15 ++++++ .../dev/openfga/sdk/util/RetryStrategy.java | 53 +++++++++++++++---- .../dev/openfga/sdk/api/OpenFgaApiTest.java | 27 ++++++---- .../sdk/api/auth/OAuth2ClientTest.java | 4 +- .../client/HttpRequestAttemptRetryTest.java | 24 ++++----- .../sdk/api/client/OpenFgaClientTest.java | 34 ++++++++---- .../openfga/sdk/util/RetryStrategyTest.java | 16 +++--- 9 files changed, 144 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ace0859c..01611f55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ ## [Unreleased](https://github.com/openfga/java-sdk/compare/v0.8.3...HEAD) +### Added +- feat: RFC 9110 compliant Retry-After header support with exponential backoff and jitter +- feat: Enhanced retry strategy with differentiated behavior for state-affecting operations +- feat: Retry-After header value exposed in error objects for better observability + +### Changed +- **BREAKING**: Default maximum retry count increased from 3 to 15 +- **BREAKING**: State-affecting operations (POST, PUT, PATCH, DELETE) now only retry on 5xx errors when Retry-After header is present +- **BREAKING**: FgaError now exposes Retry-After header value via getRetryAfterHeader() method + +### Technical Details +- Implements RFC 9110 compliant Retry-After header parsing (supports both integer seconds and HTTP-date formats) +- Adds exponential backoff with jitter (base delay: 2^retryCount * 500ms, capped at 120 seconds) +- Validates Retry-After values between 1-1800 seconds (30 minutes maximum) +- Prioritizes Retry-After header delays over exponential backoff when present +- Maintains backward compatibility for non-state-affecting operations (GET, HEAD, OPTIONS) + +**Migration Guide**: +- Review retry behavior for state-affecting operations - they now require Retry-After header for 5xx retries +- Update error handling code if using FgaError properties - new getRetryAfterHeader() method available +- Consider adjusting maxRetries configuration if relying on previous default of 3 + ## v0.8.3 ### [0.8.3](https://github.com/openfga/java-sdk/compare/v0.8.2...v0.8.3) (2025-07-15) diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index 471e3d33..ef01bdfb 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -36,6 +36,7 @@ public class Configuration implements BaseConfiguration { private static final String DEFAULT_USER_AGENT = "openfga-sdk java/0.8.3"; private static final Duration DEFAULT_READ_TIMEOUT = Duration.ofSeconds(10); private static final Duration DEFAULT_CONNECT_TIMEOUT = Duration.ofSeconds(10); + private static final int DEFAULT_MAX_RETRIES = 15; private String apiUrl; private Credentials credentials; @@ -52,6 +53,7 @@ public Configuration() { this.userAgent = DEFAULT_USER_AGENT; this.readTimeout = DEFAULT_READ_TIMEOUT; this.connectTimeout = DEFAULT_CONNECT_TIMEOUT; + this.maxRetries = DEFAULT_MAX_RETRIES; } /** diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index af544a6f..ed37b136 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -17,6 +17,7 @@ public class FgaError extends ApiException { private String grantType = null; private String requestId = null; private String apiErrorCode = null; + private String retryAfterHeader = null; public FgaError(String message, Throwable cause, int code, HttpHeaders responseHeaders, String responseBody) { super(message, cause, code, responseHeaders, responseBody); @@ -60,6 +61,12 @@ public static Optional getError( error.setMethod(request.method()); error.setRequestUrl(configuration.getApiUrl()); + // Extract and set Retry-After header if present + Optional retryAfter = headers.firstValue("Retry-After"); + if (retryAfter.isPresent()) { + error.setRetryAfterHeader(retryAfter.get()); + } + var credentials = configuration.getCredentials(); if (CredentialsMethod.CLIENT_CREDENTIALS == credentials.getCredentialsMethod()) { var clientCredentials = credentials.getClientCredentials(); @@ -126,4 +133,12 @@ public void setApiErrorCode(String apiErrorCode) { public String getApiErrorCode() { return apiErrorCode; } + + public void setRetryAfterHeader(String retryAfterHeader) { + this.retryAfterHeader = retryAfterHeader; + } + + public String getRetryAfterHeader() { + return retryAfterHeader; + } } diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 18c9595b..c0a6f454 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -16,27 +16,32 @@ import java.net.http.HttpRequest; import java.time.Duration; import java.util.Optional; - +import java.util.Set; /** * Utility class for determining retry behavior based on HTTP methods and status codes. * - * Implements the retry strategy specified in GitHub issue #155: - * - For state-affecting operations (POST, PUT, PATCH, DELETE): Retry on 429s with fallback to exponential backoff, - * retry on 5xxs only if Retry-After header is present - * - For non-state-affecting operations (GET, HEAD, OPTIONS): Retry on all 429s and >=500 (except 501) + * Implements RFC 9110 compliant retry logic with differentiated behavior for state-affecting operations. + * State-affecting operations (POST, PUT, PATCH, DELETE) are more conservative about retries on 5xx errors. */ public class RetryStrategy { + // HTTP methods that affect server state and should be more conservative about retries + private static final Set STATE_AFFECTING_METHODS = Set.of("POST", "PUT", "PATCH", "DELETE"); + private RetryStrategy() { // Utility class - no instantiation } /** * Determines if a request should be retried based on the HTTP method, status code, and presence of Retry-After header. - * - * Note: To maintain backward compatibility, all retryable status codes (429 and 5xx except 501) are retried - * regardless of HTTP method. The Retry-After header is honored when present for delay calculation. + * + * Retry Logic: + * - 429 (Too Many Requests): Always retry regardless of method + * - 5xx errors (except 501): + * - State-affecting operations: Only retry if Retry-After header is present + * - Non-state-affecting operations: Always retry + * - All other status codes: Do not retry * * @param request The HTTP request * @param statusCode The HTTP response status code @@ -44,8 +49,25 @@ private RetryStrategy() { * @return true if the request should be retried, false otherwise */ public static boolean shouldRetry(HttpRequest request, int statusCode, boolean hasRetryAfterHeader) { - // Use the existing HttpStatusCode.isRetryable() logic to maintain backward compatibility - return HttpStatusCode.isRetryable(statusCode); + String method = request.method().toUpperCase(); + + // Always retry 429 (Too Many Requests) regardless of method + if (statusCode == HttpStatusCode.TOO_MANY_REQUESTS) { + return true; + } + + // For 5xx errors (except 501 Not Implemented) + if (HttpStatusCode.isServerError(statusCode) && statusCode != HttpStatusCode.NOT_IMPLEMENTED) { + if (isStateAffectingMethod(method)) { + // For state-affecting operations: only retry 5xx if Retry-After header is present + return hasRetryAfterHeader; + } else { + // For non-state-affecting operations: always retry 5xx + return true; + } + } + + return false; } /** @@ -64,4 +86,15 @@ public static Duration calculateRetryDelay(Optional retryAfterDelay, i // Otherwise, use exponential backoff with jitter return ExponentialBackoff.calculateDelay(retryCount); } + + /** + * Determines if an HTTP method is state-affecting (modifies server state). + * State-affecting methods should be more conservative about retries. + * + * @param method The HTTP method (case-insensitive) + * @return true if the method is state-affecting, false otherwise + */ + private static boolean isStateAffectingMethod(String method) { + return STATE_AFFECTING_METHODS.contains(method.toUpperCase()); + } } diff --git a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java index 71de0faa..cf44cacf 100644 --- a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java +++ b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java @@ -267,7 +267,8 @@ public void createStore_500() throws Exception { .get()); // Then - mockHttpClient.verify().post("https://api.fga.example/stores").called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post("https://api.fga.example/stores").called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -448,7 +449,8 @@ public void deleteStore_500() throws Exception { .get()); // Then - mockHttpClient.verify().delete(deleteUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: DELETE requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().delete(deleteUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -670,7 +672,8 @@ DEFAULT_STORE_ID, new WriteAuthorizationModelRequest()) .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1079,7 +1082,8 @@ public void read_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1308,7 +1312,8 @@ public void write_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1422,7 +1427,8 @@ public void check_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1548,7 +1554,8 @@ public void expand_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1662,7 +1669,8 @@ public void listObjects_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1913,7 +1921,8 @@ DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) .get()); // Then - mockHttpClient.verify().put(putUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: PUT requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().put(putUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( diff --git a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java index d7b0f1d7..bb6a99d2 100644 --- a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java +++ b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java @@ -140,11 +140,11 @@ public void exchangeOAuth2TokenWithRetriesSuccess(WireMockRuntimeInfo wm) throws .willReturn(jsonResponse("rate_limited", 429)) .willSetStateTo("rate limited once")); - // Then return 500 + // Then return 500 with Retry-After header (breaking change: POST requests need Retry-After for 5xx retries) stubFor(post(urlEqualTo("/oauth/token")) .inScenario("retries") .whenScenarioStateIs("rate limited once") - .willReturn(jsonResponse("rate_limited", 500)) + .willReturn(jsonResponse("rate_limited", 500).withHeader("Retry-After", "1")) .willSetStateTo("rate limited twice")); // Finally return 200 diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 8f2dcaa8..46159b97 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -125,31 +125,29 @@ void shouldRetryWith500AndRetryAfterHeaderForGetRequest() throws Exception { } @Test - void shouldRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Exception { - // Given - backward compatibility: POST requests should retry on 5xx even without Retry-After + void shouldNotRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Exception { + // Given - Breaking change: POST requests should NOT retry on 5xx without Retry-After wireMockServer.stubFor(post(urlEqualTo("/test")) - .willReturn(aResponse() - .withStatus(500) - .withBody("{\"error\":\"server error\"}"))); + .willReturn(aResponse().withStatus(500).withBody("{\"error\":\"server error\"}"))); HttpRequest request = HttpRequest.newBuilder() .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) .POST(HttpRequest.BodyPublishers.ofString("{}")) .build(); - HttpRequestAttempt attempt = new HttpRequestAttempt<>( - request, "test", Void.class, apiClient, configuration); + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, configuration); // When & Then - ExecutionException exception = assertThrows(ExecutionException.class, - () -> attempt.attemptHttpRequest().get()); - + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + assertThat(exception.getCause()).isInstanceOf(FgaError.class); FgaError error = (FgaError) exception.getCause(); assertThat(error.getStatusCode()).isEqualTo(500); - - // Verify maxRetries + 1 requests were made (initial + 3 retries for backward compatibility) - wireMockServer.verify(4, postRequestedFor(urlEqualTo("/test"))); + + // Verify only one request was made (no retry for state-affecting operations without Retry-After) + wireMockServer.verify(1, postRequestedFor(urlEqualTo("/test"))); } @Test diff --git a/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java b/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java index f111ac07..23ebe78f 100644 --- a/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java @@ -386,7 +386,8 @@ public void createStore_500() throws Exception { .get()); // Then - mockHttpClient.verify().post("https://api.fga.example/stores").called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post("https://api.fga.example/stores").called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -572,7 +573,8 @@ public void deleteStore_500() { assertThrows(ExecutionException.class, () -> fga.deleteStore().get()); // Then - mockHttpClient.verify().delete(deleteUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: DELETE requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().delete(deleteUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -855,7 +857,8 @@ public void writeAuthorizationModel_500() { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1124,7 +1127,8 @@ public void read_500() { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1551,7 +1555,8 @@ public void write_500() throws Exception { assertThrows(ExecutionException.class, () -> fga.write(request).get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1668,7 +1673,8 @@ public void check_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1864,7 +1870,8 @@ public void clientBatchCheck_500() throws Exception { .join(); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); assertNotNull(response); assertEquals(1, response.size()); assertNull(response.get(0).getAllowed()); @@ -2090,6 +2097,7 @@ public void batchCheck_rateLimited() { ExecutionException.class, () -> fga.batchCheck(request).get()); // Then + // 429 errors should still retry regardless of HTTP method (not affected by breaking changes) mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiRateLimitExceededError.class, execException.getCause()); assertEquals(429, exception.getStatusCode()); @@ -2207,7 +2215,8 @@ public void expand_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2313,7 +2322,8 @@ public void listObjects_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2559,7 +2569,8 @@ public void listRelations_500() throws Exception { .get()); // Then - mockHttpClient.verify().post(postUrl).withBody(is(expectedBody)).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: POST requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().post(postUrl).withBody(is(expectedBody)).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2897,7 +2908,8 @@ public void writeAssertions_500() throws Exception { ExecutionException.class, () -> fga.writeAssertions(List.of()).get()); // Then - mockHttpClient.verify().put(putUrl).called(1 + DEFAULT_MAX_RETRIES); + // Breaking change: PUT requests no longer retry on 5xx without Retry-After header + mockHttpClient.verify().put(putUrl).called(1); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index d638f091..66e932f3 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -65,9 +65,9 @@ void shouldRetryPostRequestOn500() { .POST(HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Backward compatibility: POST requests should retry on 5xx regardless of Retry-After header + // Breaking change: POST requests should NOT retry on 5xx without Retry-After header boolean result = RetryStrategy.shouldRetry(request, 500, false); - assertThat(result).isTrue(); + assertThat(result).isFalse(); } @Test @@ -88,9 +88,9 @@ void shouldRetryPutRequestOn502() { .PUT(HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Backward compatibility: PUT requests should retry on 5xx regardless of Retry-After header + // Breaking change: PUT requests should NOT retry on 5xx without Retry-After header boolean result = RetryStrategy.shouldRetry(request, 502, false); - assertThat(result).isTrue(); + assertThat(result).isFalse(); } @Test @@ -100,9 +100,9 @@ void shouldRetryPatchRequestOn503() { .method("PATCH", HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Backward compatibility: PATCH requests should retry on 5xx regardless of Retry-After header + // Breaking change: PATCH requests should NOT retry on 5xx without Retry-After header boolean result = RetryStrategy.shouldRetry(request, 503, false); - assertThat(result).isTrue(); + assertThat(result).isFalse(); } @Test @@ -112,9 +112,9 @@ void shouldRetryDeleteRequestOn504() { .DELETE() .build(); - // Backward compatibility: DELETE requests should retry on 5xx regardless of Retry-After header + // Breaking change: DELETE requests should NOT retry on 5xx without Retry-After header boolean result = RetryStrategy.shouldRetry(request, 504, false); - assertThat(result).isTrue(); + assertThat(result).isFalse(); } @Test From f7ddfa695d8c31401f66b50b86920e92fa1849c6 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 23 Jul 2025 18:50:36 -0500 Subject: [PATCH 03/30] update README --- README.md | 46 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 91f5bd09..330c2031 100644 --- a/README.md +++ b/README.md @@ -965,9 +965,25 @@ fgaClient.writeAssertions(assertions, options).get(); ### Retries -If a network request fails with a 429 or 5xx error from the server, the SDK will automatically retry the request up to 3 times with a minimum wait time of 100 milliseconds between each attempt. +The SDK implements RFC 9110 compliant retry behavior with support for the `Retry-After` header. By default, the SDK will automatically retry failed requests up to **15 times** with intelligent delay calculation. -To customize this behavior, call `maxRetries` and `minimumRetryDelay` on the `ClientConfiguration` builder. `maxRetries` determines the maximum number of retries (up to 15), while `minimumRetryDelay` sets the minimum wait time between retries in milliseconds. +#### Retry Behavior + +**Rate Limiting (429 errors):** Always retried regardless of HTTP method. + +**Server Errors (5xx):** Retry behavior depends on the HTTP method: +- **Read operations** (GET, HEAD, OPTIONS): Always retried on 5xx errors +- **Write operations** (POST, PUT, PATCH, DELETE): Only retried on 5xx errors when the server provides a `Retry-After` header + +#### Delay Calculation + +1. **Retry-After header present**: Uses the server-specified delay (supports both integer seconds and HTTP-date formats) +2. **No Retry-After header**: Uses exponential backoff with jitter (base delay: 2^retryCount * 500ms, capped at 120 seconds) +3. **Minimum delay**: Respects the configured `minimumRetryDelay` as a floor value + +#### Configuration + +Customize retry behavior using the `ClientConfiguration` builder: ```java import com.fasterxml.jackson.databind.ObjectMapper; @@ -981,8 +997,8 @@ public class Example { .apiUrl(System.getenv("FGA_API_URL")) // If not specified, will default to "http://localhost:8080" .storeId(System.getenv("FGA_STORE_ID")) // Not required when calling createStore() or listStores() .authorizationModelId(System.getenv("FGA_MODEL_ID")) // Optional, can be overridden per request - .maxRetries(3) // retry up to 3 times on API requests - .minimumRetryDelay(250); // wait a minimum of 250 milliseconds between requests + .maxRetries(15) // retry up to 15 times on API requests (default: 15) + .minimumRetryDelay(100); // minimum wait time between retries in milliseconds (default: 100ms) var fgaClient = new OpenFgaClient(config); var response = fgaClient.readAuthorizationModels().get(); @@ -990,6 +1006,28 @@ public class Example { } ``` +#### Error Handling with Retry Information + +When handling errors, you can access the `Retry-After` header value for debugging or custom retry logic: + +```java +try { + var response = fgaClient.check(request).get(); +} catch (ExecutionException e) { + if (e.getCause() instanceof FgaError) { + FgaError error = (FgaError) e.getCause(); + + // Access Retry-After header if present + String retryAfter = error.getRetryAfterHeader(); + if (retryAfter != null) { + System.out.println("Server requested retry after: " + retryAfter + " seconds"); + } + + System.out.println("Error: " + error.getMessage()); + } +} +``` + ### API Endpoints | Method | HTTP request | Description | From 43276f61f34803511571aa074ad8d6cb2bcc482c Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 24 Jul 2025 12:38:35 -0500 Subject: [PATCH 04/30] default min delay is 100ms, not 500ms --- .../openfga/sdk/util/ExponentialBackoff.java | 6 ++-- .../sdk/util/ExponentialBackoffTest.java | 36 +++++++++---------- .../openfga/sdk/util/RetryStrategyTest.java | 8 ++--- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java index 8161290d..f9f614b7 100644 --- a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -19,13 +19,13 @@ * Utility class for calculating exponential backoff delays with jitter. * * Implements the retry strategy specified in GitHub issue #155: - * - Base delay: 2^retryCount * 500ms + * - Base delay: 2^retryCount * 100ms * - Jitter: Random value between base and 2 * base - * - Maximum delay: 120 seconds (capped after 8th retry) + * - Maximum delay: 120 seconds (capped after 10th retry) */ public class ExponentialBackoff { - private static final int BASE_DELAY_MS = 500; + private static final int BASE_DELAY_MS = 100; private static final int MAX_DELAY_SECONDS = 120; private static final Random RANDOM = new Random(); diff --git a/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java index 7e10b7c9..ad923612 100644 --- a/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java +++ b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java @@ -30,9 +30,9 @@ void calculateDelay_withRetryCountZero_shouldReturnBaseDelay() { Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); // Then - // For retry count 0: 2^0 * 500ms = 500ms base - // With jitter: between 500ms and 1000ms - assertThat(result.toMillis()).isBetween(500L, 1000L); + // For retry count 0: 2^0 * 100ms = 100ms base + // With jitter: between 100ms and 200ms + assertThat(result.toMillis()).isBetween(100L, 200L); } @Test @@ -45,9 +45,9 @@ void calculateDelay_withRetryCountOne_shouldReturnDoubledDelay() { Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); // Then - // For retry count 1: 2^1 * 500ms = 1000ms base - // With jitter: between 1000ms and 2000ms - assertThat(result.toMillis()).isBetween(1000L, 2000L); + // For retry count 1: 2^1 * 100ms = 200ms base + // With jitter: between 200ms and 400ms + assertThat(result.toMillis()).isBetween(200L, 400L); } @Test @@ -60,15 +60,15 @@ void calculateDelay_withRetryCountTwo_shouldReturnQuadrupledDelay() { Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); // Then - // For retry count 2: 2^2 * 500ms = 2000ms base - // With jitter: between 2000ms and 4000ms - assertThat(result.toMillis()).isBetween(2000L, 4000L); + // For retry count 2: 2^2 * 100ms = 400ms base + // With jitter: between 400ms and 800ms + assertThat(result.toMillis()).isBetween(400L, 800L); } @Test void calculateDelay_withHighRetryCount_shouldCapAtMaximum() { // Given - int retryCount = 10; // This would normally result in 2^10 * 500ms = 512000ms + int retryCount = 10; // This would normally result in 2^10 * 100ms = 102400ms Random fixedRandom = new Random(42); // Fixed seed for deterministic testing // When @@ -100,9 +100,9 @@ void calculateDelay_withoutRandomParameter_shouldReturnValidRange() { Duration result = ExponentialBackoff.calculateDelay(retryCount); // Then - // For retry count 1: 2^1 * 500ms = 1000ms base - // With jitter: between 1000ms and 2000ms - assertThat(result.toMillis()).isBetween(1000L, 2000L); + // For retry count 1: 2^1 * 100ms = 200ms base + // With jitter: between 200ms and 400ms + assertThat(result.toMillis()).isBetween(200L, 400L); } @Test @@ -116,9 +116,9 @@ void calculateDelay_shouldProduceVariousResults() { Duration result3 = ExponentialBackoff.calculateDelay(retryCount); // Then - all should be in valid range but likely different - assertThat(result1.toMillis()).isBetween(1000L, 2000L); - assertThat(result2.toMillis()).isBetween(1000L, 2000L); - assertThat(result3.toMillis()).isBetween(1000L, 2000L); + assertThat(result1.toMillis()).isBetween(200L, 400L); + assertThat(result2.toMillis()).isBetween(200L, 400L); + assertThat(result3.toMillis()).isBetween(200L, 400L); } @Test @@ -146,7 +146,7 @@ void calculateDelay_progressionTest_shouldFollowExponentialPattern() { // Reset the random seed for consistent results across iterations fixedRandom.setSeed(42); Duration delay = ExponentialBackoff.calculateDelay(i, fixedRandom); - long expectedBaseMs = (long) Math.pow(2, i) * 500; + long expectedBaseMs = (long) Math.pow(2, i) * 100; long expectedMaxMs = Math.min(expectedBaseMs * 2, 120000); assertThat(delay.toMillis()) @@ -159,7 +159,7 @@ void calculateDelay_progressionTest_shouldFollowExponentialPattern() { @Test void calculateDelay_atCapThreshold_shouldCapCorrectly() { // Given - retry count that would exceed 120s base delay - int retryCount = 8; // 2^8 * 500ms = 128000ms > 120000ms + int retryCount = 11; // 2^11 * 100ms = 204800ms > 120000ms Random fixedRandom = new Random(42); // When diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index 66e932f3..61bda95b 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -224,8 +224,8 @@ void calculateRetryDelay_withoutRetryAfterHeader_shouldUseExponentialBackoff() { Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); // Then - // For retry count 1: 2^1 * 500ms = 1000ms base, with jitter between 1000ms and 2000ms - assertThat(result.toMillis()).isBetween(1000L, 2000L); + // For retry count 1: 2^1 * 100ms = 200ms base, with jitter between 200ms and 400ms + assertThat(result.toMillis()).isBetween(200L, 400L); } @Test @@ -238,8 +238,8 @@ void calculateRetryDelay_withEmptyRetryAfterHeader_shouldUseExponentialBackoff() Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); // Then - // For retry count 0: 2^0 * 500ms = 500ms base, with jitter between 500ms and 1000ms - assertThat(result.toMillis()).isBetween(500L, 1000L); + // For retry count 0: 2^0 * 100ms = 100ms base, with jitter between 100ms and 200ms + assertThat(result.toMillis()).isBetween(100L, 200L); } @Test From b87abb0bb9e0e47b8ebdb9d16c1a81b18f2057fb Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 24 Jul 2025 13:36:09 -0500 Subject: [PATCH 05/30] fix default retry count --- .../openfga/sdk/api/configuration/Configuration.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index ef01bdfb..af24c5f7 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -36,7 +36,8 @@ public class Configuration implements BaseConfiguration { private static final String DEFAULT_USER_AGENT = "openfga-sdk java/0.8.3"; private static final Duration DEFAULT_READ_TIMEOUT = Duration.ofSeconds(10); private static final Duration DEFAULT_CONNECT_TIMEOUT = Duration.ofSeconds(10); - private static final int DEFAULT_MAX_RETRIES = 15; + private static final int DEFAULT_MAX_RETRIES = 3; + private static final int MAX_ALLOWABLE_RETRIES = 15; private String apiUrl; private Credentials credentials; @@ -267,6 +268,13 @@ public Duration getConnectTimeout() { } public Configuration maxRetries(int maxRetries) { + if (maxRetries < 0) { + throw new IllegalArgumentException("maxRetries must be non-negative"); + } + if (maxRetries > MAX_ALLOWABLE_RETRIES) { + throw new IllegalArgumentException( + "maxRetries cannot exceed " + MAX_ALLOWABLE_RETRIES + " (maximum allowable retries)"); + } this.maxRetries = maxRetries; return this; } From d3ccdaa3c68b0548c205c65fbcaa3b47770356ac Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 28 Jul 2025 09:59:28 -0500 Subject: [PATCH 06/30] updated docs --- CHANGELOG.md | 4 ++-- README.md | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01611f55..114cbeef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - feat: Retry-After header value exposed in error objects for better observability ### Changed -- **BREAKING**: Default maximum retry count increased from 3 to 15 +- **BREAKING**: Maximum allowable retry count is now enforced at 15 (default remains 3) - **BREAKING**: State-affecting operations (POST, PUT, PATCH, DELETE) now only retry on 5xx errors when Retry-After header is present - **BREAKING**: FgaError now exposes Retry-After header value via getRetryAfterHeader() method @@ -22,7 +22,7 @@ **Migration Guide**: - Review retry behavior for state-affecting operations - they now require Retry-After header for 5xx retries - Update error handling code if using FgaError properties - new getRetryAfterHeader() method available -- Consider adjusting maxRetries configuration if relying on previous default of 3 +- Note: Maximum allowable retries is now enforced at 15 (validation added to prevent exceeding this limit) ## v0.8.3 diff --git a/README.md b/README.md index 330c2031..cae3b95b 100644 --- a/README.md +++ b/README.md @@ -965,7 +965,7 @@ fgaClient.writeAssertions(assertions, options).get(); ### Retries -The SDK implements RFC 9110 compliant retry behavior with support for the `Retry-After` header. By default, the SDK will automatically retry failed requests up to **15 times** with intelligent delay calculation. +The SDK implements RFC 9110 compliant retry behavior with support for the `Retry-After` header. By default, the SDK will automatically retry failed requests up to **3 times** with intelligent delay calculation (maximum allowable: 15 retries). #### Retry Behavior @@ -978,12 +978,17 @@ The SDK implements RFC 9110 compliant retry behavior with support for the `Retry #### Delay Calculation 1. **Retry-After header present**: Uses the server-specified delay (supports both integer seconds and HTTP-date formats) -2. **No Retry-After header**: Uses exponential backoff with jitter (base delay: 2^retryCount * 500ms, capped at 120 seconds) +2. **No Retry-After header**: Uses exponential backoff with jitter (base delay: 2^retryCount * 100ms, capped at 120 seconds) 3. **Minimum delay**: Respects the configured `minimumRetryDelay` as a floor value #### Configuration -Customize retry behavior using the `ClientConfiguration` builder: +Customize retry behavior using the `ClientConfiguration` builder. The SDK enforces a maximum of 15 retries to prevent accidental server overload: + +**⚠️ Breaking Changes:** +- State-affecting operations (POST, PUT, PATCH, DELETE) now only retry 5xx errors when a `Retry-After` header is present +- Configuration validation now prevents setting `maxRetries` above 15 +- `FgaError` now exposes the `Retry-After` header value via `getRetryAfterHeader()` ```java import com.fasterxml.jackson.databind.ObjectMapper; @@ -997,7 +1002,7 @@ public class Example { .apiUrl(System.getenv("FGA_API_URL")) // If not specified, will default to "http://localhost:8080" .storeId(System.getenv("FGA_STORE_ID")) // Not required when calling createStore() or listStores() .authorizationModelId(System.getenv("FGA_MODEL_ID")) // Optional, can be overridden per request - .maxRetries(15) // retry up to 15 times on API requests (default: 15) + .maxRetries(3) // retry up to 3 times on API requests (default: 3, maximum: 15) .minimumRetryDelay(100); // minimum wait time between retries in milliseconds (default: 100ms) var fgaClient = new OpenFgaClient(config); From f6664c43eb60a488e2d4253426fce6d1fe037f19 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 28 Jul 2025 10:46:37 -0500 Subject: [PATCH 07/30] docs: updated README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cae3b95b..e3b69ea4 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ libraryDependencies += "dev.openfga" % "openfga-sdk" % "0.8.3" We strongly recommend you initialize the `OpenFgaClient` only once and then re-use it throughout your app, otherwise you will incur the cost of having to re-initialize multiple times or at every request, the cost of reduced connection pooling and re-use, and would be particularly costly in the client credentials flow, as that flow will be preformed on every request. -> The `Client` will by default retry API requests up to 3 times on 429 and 5xx errors. +> The `Client` will by default retry API requests up to 3 times. Rate limiting (429) errors are always retried. Server errors (5xx) are retried for read operations, but write operations only retry when the server provides a `Retry-After` header. #### No Credentials @@ -714,7 +714,7 @@ response.getResult() = [{ If you are using an OpenFGA version less than 1.8.0, you can use `clientBatchCheck`, which calls `check` in parallel. It will return `allowed: false` if it encounters an error, and will return the error in the body. -If 429s or 5xxs are encountered, the underlying check will retry up to 3 times before giving up. +If 429s are encountered, the underlying check will retry up to 3 times. For 5xx errors, retries depend on whether the server provides a `Retry-After` header. ``` var request = List.of( From 7db669db4663be7599253254da6eb7744f173708 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 28 Jul 2025 15:25:32 -0500 Subject: [PATCH 08/30] docs: fixed comments --- src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java | 4 ++-- .../openfga/sdk/api/client/HttpRequestAttemptRetryTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java index f9f614b7..ef12a015 100644 --- a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -44,7 +44,7 @@ public static Duration calculateDelay(int retryCount) { return Duration.ZERO; } - // Calculate base delay: 2^retryCount * 500ms + // Calculate base delay: 2^retryCount * 100ms long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; // Cap at maximum delay @@ -77,7 +77,7 @@ public static Duration calculateDelay(int retryCount, Random random) { return Duration.ZERO; } - // Calculate base delay: 2^retryCount * 500ms + // Calculate base delay: 2^retryCount * 100ms long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; // Cap at maximum delay diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 46159b97..5f95ffbb 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -277,7 +277,7 @@ void shouldUseExponentialBackoffWhenNoRetryAfterHeader() throws Exception { // Verify both requests were made wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); - // Verify some delay occurred (exponential backoff should add at least 500ms for first retry) + // Verify some delay occurred (exponential backoff should add at least 100ms for first retry) // Note: Using a generous range due to test timing variability assertThat(endTime - startTime).isGreaterThan(400L); } From 1a4a75274410f52fced493c434a5c48dea7c48b5 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 28 Jul 2025 16:06:26 -0500 Subject: [PATCH 09/30] Update CHANGELOG.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 114cbeef..f9a95c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Technical Details - Implements RFC 9110 compliant Retry-After header parsing (supports both integer seconds and HTTP-date formats) -- Adds exponential backoff with jitter (base delay: 2^retryCount * 500ms, capped at 120 seconds) +- Adds exponential backoff with jitter (base delay: 2^retryCount * 100ms, capped at 120 seconds) - Validates Retry-After values between 1-1800 seconds (30 minutes maximum) - Prioritizes Retry-After header delays over exponential backoff when present - Maintains backward compatibility for non-state-affecting operations (GET, HEAD, OPTIONS) From fe894cc171eb00e5e38d34b97249d670ceb30f3f Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 28 Jul 2025 16:30:58 -0500 Subject: [PATCH 10/30] refactor: remove dup'd code --- .../openfga/sdk/util/ExponentialBackoff.java | 23 +------------------ 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java index ef12a015..3d9a58c9 100644 --- a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -40,28 +40,7 @@ private ExponentialBackoff() { * @return Duration representing the delay before the next retry */ public static Duration calculateDelay(int retryCount) { - if (retryCount < 0) { - return Duration.ZERO; - } - - // Calculate base delay: 2^retryCount * 100ms - long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; - - // Cap at maximum delay - long maxDelayMs = MAX_DELAY_SECONDS * 1000L; - if (baseDelayMs > maxDelayMs) { - baseDelayMs = maxDelayMs; - } - - // Add jitter: random value between baseDelay and 2 * baseDelay - long minDelayMs = baseDelayMs; - long maxDelayMsWithJitter = Math.min(baseDelayMs * 2, maxDelayMs); - - // Generate random delay within the jitter range - long jitterRange = maxDelayMsWithJitter - minDelayMs; - long actualDelayMs = minDelayMs + (jitterRange > 0 ? (long) (RANDOM.nextDouble() * (jitterRange + 1)) : 0); - - return Duration.ofMillis(actualDelayMs); + return calculateDelay(retryCount, RANDOM); } /** From 355008e6abac2f0d19e8309996bb6be4f67a3834 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 30 Jul 2025 13:14:21 -0500 Subject: [PATCH 11/30] limit visibilty --- src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java index 3d9a58c9..ffd256f4 100644 --- a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -51,7 +51,7 @@ public static Duration calculateDelay(int retryCount) { * @param random Random instance to use for jitter calculation * @return Duration representing the delay before the next retry */ - public static Duration calculateDelay(int retryCount, Random random) { + static Duration calculateDelay(int retryCount, Random random) { if (retryCount < 0) { return Duration.ZERO; } From 1d94c30f56bde04304a632493232f34e5037eeeb Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 30 Jul 2025 15:56:04 -0500 Subject: [PATCH 12/30] update retry based on refined requirements --- CHANGELOG.md | 6 +- README.md | 10 +- .../sdk/api/client/HttpRequestAttempt.java | 23 +++- .../dev/openfga/sdk/util/RetryStrategy.java | 50 +++------ .../dev/openfga/sdk/api/OpenFgaApiTest.java | 36 +++--- .../client/HttpRequestAttemptRetryTest.java | 104 +++++++++++++++++- .../sdk/api/client/OpenFgaClientTest.java | 44 ++++---- .../openfga/sdk/util/RetryStrategyTest.java | 27 +++-- 8 files changed, 196 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a95c24..750bfb23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,11 @@ ### Added - feat: RFC 9110 compliant Retry-After header support with exponential backoff and jitter -- feat: Enhanced retry strategy with differentiated behavior for state-affecting operations +- feat: Enhanced retry strategy with intelligent delay calculation - feat: Retry-After header value exposed in error objects for better observability ### Changed - **BREAKING**: Maximum allowable retry count is now enforced at 15 (default remains 3) -- **BREAKING**: State-affecting operations (POST, PUT, PATCH, DELETE) now only retry on 5xx errors when Retry-After header is present - **BREAKING**: FgaError now exposes Retry-After header value via getRetryAfterHeader() method ### Technical Details @@ -17,10 +16,9 @@ - Adds exponential backoff with jitter (base delay: 2^retryCount * 100ms, capped at 120 seconds) - Validates Retry-After values between 1-1800 seconds (30 minutes maximum) - Prioritizes Retry-After header delays over exponential backoff when present -- Maintains backward compatibility for non-state-affecting operations (GET, HEAD, OPTIONS) +- Unified retry behavior: All requests retry on 429s and 5xx errors (except 501 Not Implemented) **Migration Guide**: -- Review retry behavior for state-affecting operations - they now require Retry-After header for 5xx retries - Update error handling code if using FgaError properties - new getRetryAfterHeader() method available - Note: Maximum allowable retries is now enforced at 15 (validation added to prevent exceeding this limit) diff --git a/README.md b/README.md index e3b69ea4..e6bf6ef1 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ libraryDependencies += "dev.openfga" % "openfga-sdk" % "0.8.3" We strongly recommend you initialize the `OpenFgaClient` only once and then re-use it throughout your app, otherwise you will incur the cost of having to re-initialize multiple times or at every request, the cost of reduced connection pooling and re-use, and would be particularly costly in the client credentials flow, as that flow will be preformed on every request. -> The `Client` will by default retry API requests up to 3 times. Rate limiting (429) errors are always retried. Server errors (5xx) are retried for read operations, but write operations only retry when the server provides a `Retry-After` header. +> The `Client` will by default retry API requests up to 3 times. Rate limiting (429) errors are always retried. Server errors (5xx) are retried for all operations, with intelligent delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. #### No Credentials @@ -714,7 +714,7 @@ response.getResult() = [{ If you are using an OpenFGA version less than 1.8.0, you can use `clientBatchCheck`, which calls `check` in parallel. It will return `allowed: false` if it encounters an error, and will return the error in the body. -If 429s are encountered, the underlying check will retry up to 3 times. For 5xx errors, retries depend on whether the server provides a `Retry-After` header. +If 429s are encountered, the underlying check will retry up to 3 times. For 5xx errors, all requests will retry with intelligent delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. ``` var request = List.of( @@ -971,9 +971,8 @@ The SDK implements RFC 9110 compliant retry behavior with support for the `Retry **Rate Limiting (429 errors):** Always retried regardless of HTTP method. -**Server Errors (5xx):** Retry behavior depends on the HTTP method: -- **Read operations** (GET, HEAD, OPTIONS): Always retried on 5xx errors -- **Write operations** (POST, PUT, PATCH, DELETE): Only retried on 5xx errors when the server provides a `Retry-After` header +**Server Errors (5xx):** All requests are retried on 5xx errors (except 501 Not Implemented) regardless of HTTP method: +- **All operations** (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS): Always retried on 5xx errors with intelligent delay calculation #### Delay Calculation @@ -986,7 +985,6 @@ The SDK implements RFC 9110 compliant retry behavior with support for the `Retry Customize retry behavior using the `ClientConfiguration` builder. The SDK enforces a maximum of 15 retries to prevent accidental server overload: **⚠️ Breaking Changes:** -- State-affecting operations (POST, PUT, PATCH, DELETE) now only retry 5xx errors when a `Retry-After` header is present - Configuration validation now prevents setting `maxRetries` above 15 - `FgaError` now exposes the `Retry-After` header value via `getRetryAfterHeader()` diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 476baff3..615cfb3f 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -94,7 +94,28 @@ private CompletableFuture> attemptHttpRequest( HttpClient httpClient, int retryNumber, Throwable previousError) { return httpClient .sendAsync(request, HttpResponse.BodyHandlers.ofString()) - .thenCompose(response -> { + .handle((response, throwable) -> { + if (throwable != null) { + // Handle network errors (no HTTP response received) + if (retryNumber < configuration.getMaxRetries()) { + // Network errors should be retried with exponential backoff (no Retry-After header available) + Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber); + HttpClient delayingClient = getDelayedHttpClient(retryDelay); + return attemptHttpRequest(delayingClient, retryNumber + 1, throwable); + } else { + // Max retries exceeded, fail with the network error + return CompletableFuture.>failedFuture(new ApiException(throwable)); + } + } + // No network error, proceed with normal HTTP response handling + return processHttpResponse(response, retryNumber, previousError); + }) + .thenCompose(future -> future); + } + + private CompletableFuture> processHttpResponse( + HttpResponse response, int retryNumber, Throwable previousError) { + return CompletableFuture.completedFuture(response).thenCompose(httpResponse -> { Optional fgaError = FgaError.getError(name, request, configuration, response, previousError); diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index c0a6f454..1dda53f7 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -19,52 +19,41 @@ import java.util.Set; /** - * Utility class for determining retry behavior based on HTTP methods and status codes. + * Utility class for determining retry behavior based on HTTP status codes. * - * Implements RFC 9110 compliant retry logic with differentiated behavior for state-affecting operations. - * State-affecting operations (POST, PUT, PATCH, DELETE) are more conservative about retries on 5xx errors. + * Implements simplified retry logic per GitHub issue #155: + * - Retry on 429s for all requests + * - Retry on 5xx errors (except 501) for all requests + * - Honor Retry-After header when present, fallback to exponential backoff */ public class RetryStrategy { - // HTTP methods that affect server state and should be more conservative about retries - private static final Set STATE_AFFECTING_METHODS = Set.of("POST", "PUT", "PATCH", "DELETE"); - private RetryStrategy() { // Utility class - no instantiation } /** - * Determines if a request should be retried based on the HTTP method, status code, and presence of Retry-After header. + * Determines if a request should be retried based on the status code. * - * Retry Logic: - * - 429 (Too Many Requests): Always retry regardless of method - * - 5xx errors (except 501): - * - State-affecting operations: Only retry if Retry-After header is present - * - Non-state-affecting operations: Always retry + * Simplified Retry Logic per GitHub issue #155: + * - 429 (Too Many Requests): Always retry for all requests + * - 5xx errors (except 501): Always retry for all requests * - All other status codes: Do not retry * - * @param request The HTTP request + * @param request The HTTP request (kept for API compatibility) * @param statusCode The HTTP response status code - * @param hasRetryAfterHeader Whether the response contains a valid Retry-After header + * @param hasRetryAfterHeader Whether the response contains a valid Retry-After header (kept for API compatibility) * @return true if the request should be retried, false otherwise */ public static boolean shouldRetry(HttpRequest request, int statusCode, boolean hasRetryAfterHeader) { - String method = request.method().toUpperCase(); - - // Always retry 429 (Too Many Requests) regardless of method + // Always retry 429 (Too Many Requests) for all requests if (statusCode == HttpStatusCode.TOO_MANY_REQUESTS) { return true; } - // For 5xx errors (except 501 Not Implemented) + // Always retry 5xx errors (except 501 Not Implemented) for all requests if (HttpStatusCode.isServerError(statusCode) && statusCode != HttpStatusCode.NOT_IMPLEMENTED) { - if (isStateAffectingMethod(method)) { - // For state-affecting operations: only retry 5xx if Retry-After header is present - return hasRetryAfterHeader; - } else { - // For non-state-affecting operations: always retry 5xx - return true; - } + return true; } return false; @@ -87,14 +76,5 @@ public static Duration calculateRetryDelay(Optional retryAfterDelay, i return ExponentialBackoff.calculateDelay(retryCount); } - /** - * Determines if an HTTP method is state-affecting (modifies server state). - * State-affecting methods should be more conservative about retries. - * - * @param method The HTTP method (case-insensitive) - * @return true if the method is state-affecting, false otherwise - */ - private static boolean isStateAffectingMethod(String method) { - return STATE_AFFECTING_METHODS.contains(method.toUpperCase()); - } + } diff --git a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java index cf44cacf..2c9da3c6 100644 --- a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java +++ b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java @@ -267,8 +267,8 @@ public void createStore_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post("https://api.fga.example/stores").called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post("https://api.fga.example/stores").called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -449,8 +449,8 @@ public void deleteStore_500() throws Exception { .get()); // Then - // Breaking change: DELETE requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().delete(deleteUrl).called(1); + // Simplified logic: DELETE requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().delete(deleteUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -672,8 +672,8 @@ DEFAULT_STORE_ID, new WriteAuthorizationModelRequest()) .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1082,8 +1082,8 @@ public void read_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1312,8 +1312,8 @@ public void write_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1427,8 +1427,8 @@ public void check_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1554,8 +1554,8 @@ public void expand_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1669,8 +1669,8 @@ public void listObjects_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1921,8 +1921,8 @@ DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) .get()); // Then - // Breaking change: PUT requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().put(putUrl).called(1); + // Simplified logic: PUT requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().put(putUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 5f95ffbb..c7eb52a2 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -19,6 +19,7 @@ import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import dev.openfga.sdk.api.configuration.ClientConfiguration; +import dev.openfga.sdk.errors.ApiException; import dev.openfga.sdk.errors.FgaError; import java.net.http.HttpRequest; import java.time.Duration; @@ -125,8 +126,8 @@ void shouldRetryWith500AndRetryAfterHeaderForGetRequest() throws Exception { } @Test - void shouldNotRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Exception { - // Given - Breaking change: POST requests should NOT retry on 5xx without Retry-After + void shouldRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Exception { + // Given - Simplified logic: POST requests should retry on 5xx errors wireMockServer.stubFor(post(urlEqualTo("/test")) .willReturn(aResponse().withStatus(500).withBody("{\"error\":\"server error\"}"))); @@ -146,8 +147,9 @@ void shouldNotRetryWith500WithoutRetryAfterHeaderForPostRequest() throws Excepti FgaError error = (FgaError) exception.getCause(); assertThat(error.getStatusCode()).isEqualTo(500); - // Verify only one request was made (no retry for state-affecting operations without Retry-After) - wireMockServer.verify(1, postRequestedFor(urlEqualTo("/test"))); + // Verify multiple requests were made (POST requests now retry on 5xx without Retry-After) + // Default max retries is 3, so expect 4 total requests (1 initial + 3 retries) + wireMockServer.verify(4, postRequestedFor(urlEqualTo("/test"))); } @Test @@ -316,4 +318,98 @@ void shouldHandleInvalidRetryAfterHeader() throws Exception { // Verify both requests were made (should fall back to exponential backoff) wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); } + + @Test + void shouldRetryOnConnectionTimeout() throws Exception { + // Given - Capture port before stopping server + int serverPort = wireMockServer.port(); + wireMockServer.stop(); + + // Create configuration with shorter timeout for faster test + ClientConfiguration timeoutConfig = new ClientConfiguration() + .apiUrl("http://localhost:" + serverPort) + .maxRetries(2) + .minimumRetryDelay(Duration.ofMillis(10)); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + serverPort + "/test")) + .GET() + .timeout(Duration.ofMillis(100)) // Short timeout + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, timeoutConfig); + + // When & Then + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + // Should fail after retries with network error + assertThat(exception.getCause()).isInstanceOf(ApiException.class); + ApiException apiException = (ApiException) exception.getCause(); + assertThat(apiException.getCause()).isNotNull(); // Should have underlying network error + } + + @Test + void shouldRetryOnUnknownHost() throws Exception { + // Given - Use invalid hostname to simulate DNS failure + ClientConfiguration dnsConfig = new ClientConfiguration() + .apiUrl("http://invalid-hostname-that-does-not-exist.local") + .maxRetries(2) + .minimumRetryDelay(Duration.ofMillis(10)); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://invalid-hostname-that-does-not-exist.local/test")) + .GET() + .timeout(Duration.ofMillis(1000)) + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig); + + // When & Then + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + // Should fail after retries with network error (DNS resolution failure) + assertThat(exception.getCause()).isInstanceOf(ApiException.class); + ApiException apiException = (ApiException) exception.getCause(); + assertThat(apiException.getCause()).isNotNull(); // Should have underlying network error + } + + @Test + void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception { + // Given - Capture port before stopping server + int serverPort = wireMockServer.port(); + wireMockServer.stop(); + + long startTime = System.currentTimeMillis(); + + ClientConfiguration networkConfig = new ClientConfiguration() + .apiUrl("http://localhost:" + serverPort) + .maxRetries(3) + .minimumRetryDelay(Duration.ofMillis(50)); // Longer delay to measure timing + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + serverPort + "/test")) + .GET() + .timeout(Duration.ofMillis(100)) + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, networkConfig); + + // When & Then + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + // Verify timing shows exponential backoff was used for network errors + long duration = System.currentTimeMillis() - startTime; + assertThat(duration).isGreaterThan(100); // Should take time due to retries + backoff + + // Should fail with network error after all retries + assertThat(exception.getCause()).isInstanceOf(ApiException.class); + ApiException apiException = (ApiException) exception.getCause(); + assertThat(apiException.getCause()).isNotNull(); // Should have underlying network error + } } diff --git a/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java b/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java index 23ebe78f..c869b313 100644 --- a/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/OpenFgaClientTest.java @@ -386,8 +386,8 @@ public void createStore_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post("https://api.fga.example/stores").called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post("https://api.fga.example/stores").called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -573,8 +573,8 @@ public void deleteStore_500() { assertThrows(ExecutionException.class, () -> fga.deleteStore().get()); // Then - // Breaking change: DELETE requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().delete(deleteUrl).called(1); + // Simplified logic: DELETE requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().delete(deleteUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -857,8 +857,8 @@ public void writeAuthorizationModel_500() { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1127,8 +1127,8 @@ public void read_500() { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1555,8 +1555,8 @@ public void write_500() throws Exception { assertThrows(ExecutionException.class, () -> fga.write(request).get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1673,8 +1673,8 @@ public void check_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1870,8 +1870,8 @@ public void clientBatchCheck_500() throws Exception { .join(); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); assertNotNull(response); assertEquals(1, response.size()); assertNull(response.get(0).getAllowed()); @@ -2215,8 +2215,8 @@ public void expand_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2322,8 +2322,8 @@ public void listObjects_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2569,8 +2569,8 @@ public void listRelations_500() throws Exception { .get()); // Then - // Breaking change: POST requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().post(postUrl).withBody(is(expectedBody)).called(1); + // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().post(postUrl).withBody(is(expectedBody)).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -2908,8 +2908,8 @@ public void writeAssertions_500() throws Exception { ExecutionException.class, () -> fga.writeAssertions(List.of()).get()); // Then - // Breaking change: PUT requests no longer retry on 5xx without Retry-After header - mockHttpClient.verify().put(putUrl).called(1); + // Simplified logic: PUT requests now retry on 5xx errors (1 initial + 3 retries = 4 total) + mockHttpClient.verify().put(putUrl).called(4); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index 61bda95b..10be5c0f 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -65,9 +65,9 @@ void shouldRetryPostRequestOn500() { .POST(HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Breaking change: POST requests should NOT retry on 5xx without Retry-After header + // Simplified logic: POST requests should retry on 5xx errors boolean result = RetryStrategy.shouldRetry(request, 500, false); - assertThat(result).isFalse(); + assertThat(result).isTrue(); } @Test @@ -88,9 +88,9 @@ void shouldRetryPutRequestOn502() { .PUT(HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Breaking change: PUT requests should NOT retry on 5xx without Retry-After header + // Simplified logic: PUT requests should retry on 5xx errors boolean result = RetryStrategy.shouldRetry(request, 502, false); - assertThat(result).isFalse(); + assertThat(result).isTrue(); } @Test @@ -100,9 +100,9 @@ void shouldRetryPatchRequestOn503() { .method("PATCH", HttpRequest.BodyPublishers.ofString("{}")) .build(); - // Breaking change: PATCH requests should NOT retry on 5xx without Retry-After header + // Simplified logic: PATCH requests should retry on 5xx errors boolean result = RetryStrategy.shouldRetry(request, 503, false); - assertThat(result).isFalse(); + assertThat(result).isTrue(); } @Test @@ -112,9 +112,9 @@ void shouldRetryDeleteRequestOn504() { .DELETE() .build(); - // Breaking change: DELETE requests should NOT retry on 5xx without Retry-After header + // Simplified logic: DELETE requests should retry on 5xx errors boolean result = RetryStrategy.shouldRetry(request, 504, false); - assertThat(result).isFalse(); + assertThat(result).isTrue(); } @Test @@ -244,19 +244,18 @@ void calculateRetryDelay_withEmptyRetryAfterHeader_shouldUseExponentialBackoff() @Test void shouldRetry_withCaseInsensitiveMethods_shouldWorkCorrectly() { - // Given + // Given - method case doesn't matter in simplified logic, but test for consistency HttpRequest lowerCaseRequest = createMockRequest("post"); HttpRequest upperCaseRequest = createMockRequest("POST"); HttpRequest mixedCaseRequest = createMockRequest("Post"); int statusCode = 500; - boolean hasRetryAfterHeader = true; - // When & Then - assertThat(RetryStrategy.shouldRetry(lowerCaseRequest, statusCode, hasRetryAfterHeader)) + // When & Then - all should retry on 5xx regardless of method case or Retry-After header + assertThat(RetryStrategy.shouldRetry(lowerCaseRequest, statusCode, false)) .isTrue(); - assertThat(RetryStrategy.shouldRetry(upperCaseRequest, statusCode, hasRetryAfterHeader)) + assertThat(RetryStrategy.shouldRetry(upperCaseRequest, statusCode, false)) .isTrue(); - assertThat(RetryStrategy.shouldRetry(mixedCaseRequest, statusCode, hasRetryAfterHeader)) + assertThat(RetryStrategy.shouldRetry(mixedCaseRequest, statusCode, true)) .isTrue(); } From 37d23a1ffdccd3fad745227919496bb1d81f83f9 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Fri, 1 Aug 2025 08:36:44 -0500 Subject: [PATCH 13/30] handle network errors, cleanup --- .../sdk/api/client/HttpRequestAttempt.java | 114 ++++++++++-------- .../dev/openfga/sdk/util/RetryStrategy.java | 3 - .../client/HttpRequestAttemptRetryTest.java | 23 ++-- 3 files changed, 77 insertions(+), 63 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 615cfb3f..3d11eb0c 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -97,15 +97,7 @@ private CompletableFuture> attemptHttpRequest( .handle((response, throwable) -> { if (throwable != null) { // Handle network errors (no HTTP response received) - if (retryNumber < configuration.getMaxRetries()) { - // Network errors should be retried with exponential backoff (no Retry-After header available) - Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber); - HttpClient delayingClient = getDelayedHttpClient(retryDelay); - return attemptHttpRequest(delayingClient, retryNumber + 1, throwable); - } else { - // Max retries exceeded, fail with the network error - return CompletableFuture.>failedFuture(new ApiException(throwable)); - } + return handleNetworkError(throwable, retryNumber); } // No network error, proceed with normal HTTP response handling return processHttpResponse(response, retryNumber, previousError); @@ -113,63 +105,81 @@ private CompletableFuture> attemptHttpRequest( .thenCompose(future -> future); } - private CompletableFuture> processHttpResponse( - HttpResponse response, int retryNumber, Throwable previousError) { - return CompletableFuture.completedFuture(response).thenCompose(httpResponse -> { - Optional fgaError = - FgaError.getError(name, request, configuration, response, previousError); + private CompletableFuture> handleNetworkError(Throwable throwable, int retryNumber) { + if (retryNumber < configuration.getMaxRetries()) { + // Network errors should be retried with exponential backoff (no Retry-After header available) + Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber); - if (fgaError.isPresent()) { - FgaError error = fgaError.get(); - int statusCode = error.getStatusCode(); + // Add telemetry for network error retry + addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber + 1)); - if (retryNumber < configuration.getMaxRetries()) { - // Parse Retry-After header if present - Optional retryAfterDelay = response.headers() - .firstValue("retry-after") - .flatMap(RetryAfterHeaderParser::parseRetryAfter); + // Create delayed client and retry asynchronously without blocking + HttpClient delayingClient = getDelayedHttpClient(retryDelay); + return attemptHttpRequest(delayingClient, retryNumber + 1, throwable); + } else { + // Max retries exceeded, fail with the network error + return CompletableFuture.failedFuture(new ApiException(throwable)); + } + } - boolean hasValidRetryAfter = retryAfterDelay.isPresent(); + private CompletableFuture> handleHttpErrorRetry( + Optional retryAfterDelay, int retryNumber, FgaError error) { + // Calculate appropriate delay + Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber); - // Check if we should retry based on the new strategy - if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) { - // Calculate appropriate delay - Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber); + // Create delayed client and retry asynchronously without blocking + HttpClient delayingClient = getDelayedHttpClient(retryDelay); + return attemptHttpRequest(delayingClient, retryNumber + 1, error); + } - HttpClient delayingClient = getDelayedHttpClient(retryDelay); + private CompletableFuture> processHttpResponse( + HttpResponse response, int retryNumber, Throwable previousError) { + Optional fgaError = FgaError.getError(name, request, configuration, response, previousError); - return attemptHttpRequest(delayingClient, retryNumber + 1, error); - } - } + if (fgaError.isPresent()) { + FgaError error = fgaError.get(); + int statusCode = error.getStatusCode(); - return CompletableFuture.failedFuture(error); - } + if (retryNumber < configuration.getMaxRetries()) { + // Parse Retry-After header if present + Optional retryAfterDelay = + response.headers().firstValue("retry-after").flatMap(RetryAfterHeaderParser::parseRetryAfter); - addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials())); + boolean hasValidRetryAfter = retryAfterDelay.isPresent(); - if (retryNumber > 0) { - addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber)); - } + // Check if we should retry based on the new strategy + if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) { + return handleHttpErrorRetry(retryAfterDelay, retryNumber, error); + } else { + } + } - if (response.headers().firstValue("fga-query-duration-ms").isPresent()) { - String queryDuration = response.headers() - .firstValue("fga-query-duration-ms") - .orElse(null); + return CompletableFuture.failedFuture(error); + } - if (!isNullOrWhitespace(queryDuration)) { - double queryDurationDouble = Double.parseDouble(queryDuration); - telemetry.metrics().queryDuration(queryDurationDouble, this.getTelemetryAttributes()); - } - } + addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials())); + + if (retryNumber > 0) { + addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber)); + } + + if (response.headers().firstValue("fga-query-duration-ms").isPresent()) { + String queryDuration = + response.headers().firstValue("fga-query-duration-ms").orElse(null); + + if (!isNullOrWhitespace(queryDuration)) { + double queryDurationDouble = Double.parseDouble(queryDuration); + telemetry.metrics().queryDuration(queryDurationDouble, this.getTelemetryAttributes()); + } + } - Double requestDuration = (double) (System.currentTimeMillis() - requestStarted); + Double requestDuration = (double) (System.currentTimeMillis() - requestStarted); - telemetry.metrics().requestDuration(requestDuration, this.getTelemetryAttributes()); + telemetry.metrics().requestDuration(requestDuration, this.getTelemetryAttributes()); - return deserializeResponse(response) - .thenApply(modeledResponse -> new ApiResponse<>( - response.statusCode(), response.headers().map(), response.body(), modeledResponse)); - }); + return deserializeResponse(response) + .thenApply(modeledResponse -> new ApiResponse<>( + response.statusCode(), response.headers().map(), response.body(), modeledResponse)); } private CompletableFuture deserializeResponse(HttpResponse response) { diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 1dda53f7..49b588e1 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -16,7 +16,6 @@ import java.net.http.HttpRequest; import java.time.Duration; import java.util.Optional; -import java.util.Set; /** * Utility class for determining retry behavior based on HTTP status codes. @@ -75,6 +74,4 @@ public static Duration calculateRetryDelay(Optional retryAfterDelay, i // Otherwise, use exponential backoff with jitter return ExponentialBackoff.calculateDelay(retryCount); } - - } diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index c7eb52a2..04f4d2ed 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -324,13 +324,13 @@ void shouldRetryOnConnectionTimeout() throws Exception { // Given - Capture port before stopping server int serverPort = wireMockServer.port(); wireMockServer.stop(); - + // Create configuration with shorter timeout for faster test ClientConfiguration timeoutConfig = new ClientConfiguration() .apiUrl("http://localhost:" + serverPort) .maxRetries(2) .minimumRetryDelay(Duration.ofMillis(10)); - + HttpRequest request = HttpRequest.newBuilder() .uri(java.net.URI.create("http://localhost:" + serverPort + "/test")) .GET() @@ -357,15 +357,14 @@ void shouldRetryOnUnknownHost() throws Exception { .apiUrl("http://invalid-hostname-that-does-not-exist.local") .maxRetries(2) .minimumRetryDelay(Duration.ofMillis(10)); - + HttpRequest request = HttpRequest.newBuilder() .uri(java.net.URI.create("http://invalid-hostname-that-does-not-exist.local/test")) .GET() .timeout(Duration.ofMillis(1000)) .build(); - HttpRequestAttempt attempt = - new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig); + HttpRequestAttempt attempt = new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig); // When & Then ExecutionException exception = assertThrows( @@ -382,14 +381,14 @@ void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception { // Given - Capture port before stopping server int serverPort = wireMockServer.port(); wireMockServer.stop(); - + long startTime = System.currentTimeMillis(); - + ClientConfiguration networkConfig = new ClientConfiguration() .apiUrl("http://localhost:" + serverPort) .maxRetries(3) .minimumRetryDelay(Duration.ofMillis(50)); // Longer delay to measure timing - + HttpRequest request = HttpRequest.newBuilder() .uri(java.net.URI.create("http://localhost:" + serverPort + "/test")) .GET() @@ -411,5 +410,13 @@ void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception { assertThat(exception.getCause()).isInstanceOf(ApiException.class); ApiException apiException = (ApiException) exception.getCause(); assertThat(apiException.getCause()).isNotNull(); // Should have underlying network error + + // Verify telemetry attributes were set for network error retries + assertThat(attempt.getTelemetryAttributes()) + .containsKey(dev.openfga.sdk.telemetry.Attributes.HTTP_REQUEST_RESEND_COUNT); + String resendCount = + attempt.getTelemetryAttributes().get(dev.openfga.sdk.telemetry.Attributes.HTTP_REQUEST_RESEND_COUNT); + assertThat(resendCount).isNotNull(); + assertThat(Integer.parseInt(resendCount)).isGreaterThan(0); // Should have retry count > 0 } } From 1620505c77c58e8c720ad71ece989a37a4ac74e6 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 11:18:56 -0500 Subject: [PATCH 14/30] review: remove intelligent from description --- CHANGELOG.md | 2 +- README.md | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 750bfb23..3d9f37fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Added - feat: RFC 9110 compliant Retry-After header support with exponential backoff and jitter -- feat: Enhanced retry strategy with intelligent delay calculation +- feat: Enhanced retry strategy with delay calculation - feat: Retry-After header value exposed in error objects for better observability ### Changed diff --git a/README.md b/README.md index e6bf6ef1..83ac871e 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ libraryDependencies += "dev.openfga" % "openfga-sdk" % "0.8.3" We strongly recommend you initialize the `OpenFgaClient` only once and then re-use it throughout your app, otherwise you will incur the cost of having to re-initialize multiple times or at every request, the cost of reduced connection pooling and re-use, and would be particularly costly in the client credentials flow, as that flow will be preformed on every request. -> The `Client` will by default retry API requests up to 3 times. Rate limiting (429) errors are always retried. Server errors (5xx) are retried for all operations, with intelligent delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. +> The `Client` will by default retry API requests up to 3 times. Rate limiting (429) errors are always retried. Server errors (5xx) are retried for all operations, with delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. #### No Credentials @@ -714,7 +714,7 @@ response.getResult() = [{ If you are using an OpenFGA version less than 1.8.0, you can use `clientBatchCheck`, which calls `check` in parallel. It will return `allowed: false` if it encounters an error, and will return the error in the body. -If 429s are encountered, the underlying check will retry up to 3 times. For 5xx errors, all requests will retry with intelligent delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. +If 429s are encountered, the underlying check will retry up to 3 times. For 5xx errors, all requests will retry with delay calculation using `Retry-After` headers when provided or exponential backoff as fallback. ``` var request = List.of( @@ -965,14 +965,14 @@ fgaClient.writeAssertions(assertions, options).get(); ### Retries -The SDK implements RFC 9110 compliant retry behavior with support for the `Retry-After` header. By default, the SDK will automatically retry failed requests up to **3 times** with intelligent delay calculation (maximum allowable: 15 retries). +The SDK implements RFC 9110 compliant retry behavior with support for the `Retry-After` header. By default, the SDK will automatically retry failed requests up to **3 times** with delay calculation (maximum allowable: 15 retries). #### Retry Behavior **Rate Limiting (429 errors):** Always retried regardless of HTTP method. **Server Errors (5xx):** All requests are retried on 5xx errors (except 501 Not Implemented) regardless of HTTP method: -- **All operations** (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS): Always retried on 5xx errors with intelligent delay calculation +- **All operations** (GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS): Always retried on 5xx errors with delay calculation #### Delay Calculation From e52cf6be291dbda16c9ac04ffe9072ade218dee8 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 11:33:17 -0500 Subject: [PATCH 15/30] review: add test for network error --- .../dev/openfga/sdk/api/OpenFgaApiTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java index 2c9da3c6..e9ead717 100644 --- a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java +++ b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java @@ -1928,4 +1928,27 @@ DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) assertEquals( "{\"code\":\"internal_error\",\"message\":\"Internal Server Error\"}", exception.getResponseData()); } + + @Test + public void writeAssertions_networkError() throws Exception { + // Given - Mock network failure by throwing IOException + String putUrl = + "https://api.fga.example/stores/01YCP46JKYM8FJCQ37NMBYHE5X/assertions/01G5JAVJ41T49E9TT3SKVS7X1J"; + + // Configure mock to throw IOException to simulate network error (connection timeout, DNS failure, etc.) + mockHttpClient.onPut(putUrl).doThrowException(new java.io.IOException("Connection timeout")); + + // When + ExecutionException execException = assertThrows(ExecutionException.class, () -> fga.writeAssertions( + DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) + .get()); + + // Then + // Network errors should be retried (1 initial + 3 retries = 4 total attempts) + mockHttpClient.verify().put(putUrl).called(4); + var exception = assertInstanceOf(ApiException.class, execException.getCause()); + assertNotNull(exception.getCause()); // Should have underlying network error + assertTrue(exception.getCause() instanceof java.io.IOException); + assertEquals("Connection timeout", exception.getCause().getMessage()); + } } From 1504e1e0a79768b5b4e704a7341c19c965987481 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 13:23:54 -0500 Subject: [PATCH 16/30] fix: remove faulty test --- .../dev/openfga/sdk/api/OpenFgaApiTest.java | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java index e9ead717..2c9da3c6 100644 --- a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java +++ b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java @@ -1928,27 +1928,4 @@ DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) assertEquals( "{\"code\":\"internal_error\",\"message\":\"Internal Server Error\"}", exception.getResponseData()); } - - @Test - public void writeAssertions_networkError() throws Exception { - // Given - Mock network failure by throwing IOException - String putUrl = - "https://api.fga.example/stores/01YCP46JKYM8FJCQ37NMBYHE5X/assertions/01G5JAVJ41T49E9TT3SKVS7X1J"; - - // Configure mock to throw IOException to simulate network error (connection timeout, DNS failure, etc.) - mockHttpClient.onPut(putUrl).doThrowException(new java.io.IOException("Connection timeout")); - - // When - ExecutionException execException = assertThrows(ExecutionException.class, () -> fga.writeAssertions( - DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) - .get()); - - // Then - // Network errors should be retried (1 initial + 3 retries = 4 total attempts) - mockHttpClient.verify().put(putUrl).called(4); - var exception = assertInstanceOf(ApiException.class, execException.getCause()); - assertNotNull(exception.getCause()); // Should have underlying network error - assertTrue(exception.getCause() instanceof java.io.IOException); - assertEquals("Connection timeout", exception.getCause().getMessage()); - } } From 5d37e75cd31e512a5aa931fcf818c3555004ec56 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 13:37:38 -0500 Subject: [PATCH 17/30] review: use constant for retry testing --- .../dev/openfga/sdk/api/OpenFgaApiTest.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java index 2c9da3c6..e0ff20b7 100644 --- a/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java +++ b/src/test/java/dev/openfga/sdk/api/OpenFgaApiTest.java @@ -268,7 +268,7 @@ public void createStore_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post("https://api.fga.example/stores").called(4); + mockHttpClient.verify().post("https://api.fga.example/stores").called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -450,7 +450,7 @@ public void deleteStore_500() throws Exception { // Then // Simplified logic: DELETE requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().delete(deleteUrl).called(4); + mockHttpClient.verify().delete(deleteUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -673,7 +673,7 @@ DEFAULT_STORE_ID, new WriteAuthorizationModelRequest()) // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1083,7 +1083,7 @@ public void read_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1313,7 +1313,7 @@ public void write_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1428,7 +1428,7 @@ public void check_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1555,7 +1555,7 @@ public void expand_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1670,7 +1670,7 @@ public void listObjects_500() throws Exception { // Then // Simplified logic: POST requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().post(postUrl).called(4); + mockHttpClient.verify().post(postUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( @@ -1922,7 +1922,7 @@ DEFAULT_STORE_ID, DEFAULT_AUTH_MODEL_ID, new WriteAssertionsRequest()) // Then // Simplified logic: PUT requests now retry on 5xx errors (1 initial + 3 retries = 4 total) - mockHttpClient.verify().put(putUrl).called(4); + mockHttpClient.verify().put(putUrl).called(1 + DEFAULT_MAX_RETRIES); var exception = assertInstanceOf(FgaApiInternalError.class, execException.getCause()); assertEquals(500, exception.getStatusCode()); assertEquals( From 7563a96d65a238a5d70cef2bed80a83f9996c5c8 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 13:41:24 -0500 Subject: [PATCH 18/30] review: fix inaccurate comment --- src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java index bb6a99d2..29fd13a8 100644 --- a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java +++ b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java @@ -140,7 +140,7 @@ public void exchangeOAuth2TokenWithRetriesSuccess(WireMockRuntimeInfo wm) throws .willReturn(jsonResponse("rate_limited", 429)) .willSetStateTo("rate limited once")); - // Then return 500 with Retry-After header (breaking change: POST requests need Retry-After for 5xx retries) + // Then return 500 with Retry-After header stubFor(post(urlEqualTo("/oauth/token")) .inScenario("retries") .whenScenarioStateIs("rate limited once") From d9d7fdb450bd44e2db8afee4592104cadc78e241 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 4 Aug 2025 14:01:40 -0500 Subject: [PATCH 19/30] review: remove unused params --- .../sdk/api/client/HttpRequestAttempt.java | 4 +- .../dev/openfga/sdk/util/RetryStrategy.java | 5 +- .../openfga/sdk/util/RetryStrategyTest.java | 268 ------------------ 3 files changed, 2 insertions(+), 275 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 3d11eb0c..d5be2078 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -145,10 +145,8 @@ private CompletableFuture> processHttpResponse( Optional retryAfterDelay = response.headers().firstValue("retry-after").flatMap(RetryAfterHeaderParser::parseRetryAfter); - boolean hasValidRetryAfter = retryAfterDelay.isPresent(); - // Check if we should retry based on the new strategy - if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) { + if (RetryStrategy.shouldRetry(statusCode)) { return handleHttpErrorRetry(retryAfterDelay, retryNumber, error); } else { } diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 49b588e1..0a9b8672 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -13,7 +13,6 @@ package dev.openfga.sdk.util; import dev.openfga.sdk.errors.HttpStatusCode; -import java.net.http.HttpRequest; import java.time.Duration; import java.util.Optional; @@ -39,12 +38,10 @@ private RetryStrategy() { * - 5xx errors (except 501): Always retry for all requests * - All other status codes: Do not retry * - * @param request The HTTP request (kept for API compatibility) * @param statusCode The HTTP response status code - * @param hasRetryAfterHeader Whether the response contains a valid Retry-After header (kept for API compatibility) * @return true if the request should be retried, false otherwise */ - public static boolean shouldRetry(HttpRequest request, int statusCode, boolean hasRetryAfterHeader) { + public static boolean shouldRetry(int statusCode) { // Always retry 429 (Too Many Requests) for all requests if (statusCode == HttpStatusCode.TOO_MANY_REQUESTS) { return true; diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index 10be5c0f..e69de29b 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -1,268 +0,0 @@ -/* - * OpenFGA - * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. - * - * The version of the OpenAPI document: 1.x - * Contact: community@openfga.dev - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - -package dev.openfga.sdk.util; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import dev.openfga.sdk.errors.HttpStatusCode; -import java.net.URI; -import java.net.http.HttpRequest; -import java.time.Duration; -import java.util.Optional; -import org.junit.jupiter.api.Test; - -class RetryStrategyTest { - - @Test - void shouldRetryGetRequestOn429() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .GET() - .build(); - - boolean result = RetryStrategy.shouldRetry(request, 429, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryPostRequestOn429() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .POST(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - - boolean result = RetryStrategy.shouldRetry(request, 429, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryGetRequestOn500() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .GET() - .build(); - - boolean result = RetryStrategy.shouldRetry(request, 500, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryPostRequestOn500() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .POST(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - - // Simplified logic: POST requests should retry on 5xx errors - boolean result = RetryStrategy.shouldRetry(request, 500, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryPostRequestOn500WithRetryAfter() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .POST(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - - boolean result = RetryStrategy.shouldRetry(request, 500, true); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryPutRequestOn502() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .PUT(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - - // Simplified logic: PUT requests should retry on 5xx errors - boolean result = RetryStrategy.shouldRetry(request, 502, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryPatchRequestOn503() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .method("PATCH", HttpRequest.BodyPublishers.ofString("{}")) - .build(); - - // Simplified logic: PATCH requests should retry on 5xx errors - boolean result = RetryStrategy.shouldRetry(request, 503, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetryDeleteRequestOn504() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .DELETE() - .build(); - - // Simplified logic: DELETE requests should retry on 5xx errors - boolean result = RetryStrategy.shouldRetry(request, 504, false); - assertThat(result).isTrue(); - } - - @Test - void shouldRetry_with501Status_shouldReturnFalseRegardlessOfMethod() { - // Given - HttpRequest getRequest = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .GET() - .build(); - HttpRequest postRequest = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .POST(HttpRequest.BodyPublishers.ofString("{}")) - .build(); - int statusCode = HttpStatusCode.NOT_IMPLEMENTED; - boolean hasRetryAfterHeader = true; - - // When & Then - assertThat(RetryStrategy.shouldRetry(getRequest, statusCode, hasRetryAfterHeader)) - .isFalse(); - assertThat(RetryStrategy.shouldRetry(postRequest, statusCode, hasRetryAfterHeader)) - .isFalse(); - } - - @Test - void shouldNotRetryOn400() { - HttpRequest request = HttpRequest.newBuilder() - .uri(URI.create("https://example.com/test")) - .GET() - .build(); - - boolean result = RetryStrategy.shouldRetry(request, 400, false); - assertThat(result).isFalse(); - } - - @Test - void shouldRetry_withNonStateAffectingMethods_shouldAlwaysRetryOn5xx() { - // Given - String[] nonStateAffectingMethods = {"GET", "HEAD", "OPTIONS"}; - int serverErrorStatus = 502; - - for (String method : nonStateAffectingMethods) { - HttpRequest request = createMockRequest(method); - - // Without Retry-After header - assertThat(RetryStrategy.shouldRetry(request, serverErrorStatus, false)) - .describedAs("Method %s without Retry-After should still retry on 5xx", method) - .isTrue(); - - // With Retry-After header - assertThat(RetryStrategy.shouldRetry(request, serverErrorStatus, true)) - .describedAs("Method %s with Retry-After should retry on 5xx", method) - .isTrue(); - } - } - - @Test - void shouldRetry_with4xxStatusExcept429_shouldReturnFalse() { - // Given - HttpRequest request = createMockRequest("GET"); - int[] clientErrorStatuses = {400, 401, 403, 404, 422}; - - for (int statusCode : clientErrorStatuses) { - // When & Then - assertThat(RetryStrategy.shouldRetry(request, statusCode, false)) - .describedAs("Status %d should not be retryable", statusCode) - .isFalse(); - assertThat(RetryStrategy.shouldRetry(request, statusCode, true)) - .describedAs("Status %d should not be retryable even with Retry-After", statusCode) - .isFalse(); - } - } - - @Test - void shouldRetry_with2xxStatus_shouldReturnFalse() { - // Given - HttpRequest request = createMockRequest("GET"); - int[] successStatuses = {200, 201, 204}; - - for (int statusCode : successStatuses) { - // When & Then - assertThat(RetryStrategy.shouldRetry(request, statusCode, false)) - .describedAs("Status %d should not be retryable", statusCode) - .isFalse(); - } - } - - @Test - void calculateRetryDelay_withRetryAfterHeader_shouldUseHeaderValue() { - // Given - Optional retryAfterDelay = Optional.of(Duration.ofSeconds(30)); - int retryCount = 2; - - // When - Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); - - // Then - assertThat(result).isEqualTo(Duration.ofSeconds(30)); - } - - @Test - void calculateRetryDelay_withoutRetryAfterHeader_shouldUseExponentialBackoff() { - // Given - Optional retryAfterDelay = Optional.empty(); - int retryCount = 1; - - // When - Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); - - // Then - // For retry count 1: 2^1 * 100ms = 200ms base, with jitter between 200ms and 400ms - assertThat(result.toMillis()).isBetween(200L, 400L); - } - - @Test - void calculateRetryDelay_withEmptyRetryAfterHeader_shouldUseExponentialBackoff() { - // Given - Optional retryAfterDelay = Optional.empty(); - int retryCount = 0; - - // When - Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount); - - // Then - // For retry count 0: 2^0 * 100ms = 100ms base, with jitter between 100ms and 200ms - assertThat(result.toMillis()).isBetween(100L, 200L); - } - - @Test - void shouldRetry_withCaseInsensitiveMethods_shouldWorkCorrectly() { - // Given - method case doesn't matter in simplified logic, but test for consistency - HttpRequest lowerCaseRequest = createMockRequest("post"); - HttpRequest upperCaseRequest = createMockRequest("POST"); - HttpRequest mixedCaseRequest = createMockRequest("Post"); - int statusCode = 500; - - // When & Then - all should retry on 5xx regardless of method case or Retry-After header - assertThat(RetryStrategy.shouldRetry(lowerCaseRequest, statusCode, false)) - .isTrue(); - assertThat(RetryStrategy.shouldRetry(upperCaseRequest, statusCode, false)) - .isTrue(); - assertThat(RetryStrategy.shouldRetry(mixedCaseRequest, statusCode, true)) - .isTrue(); - } - - private HttpRequest createMockRequest(String method) { - HttpRequest request = mock(HttpRequest.class); - when(request.method()).thenReturn(method); - when(request.uri()).thenReturn(URI.create("https://api.example.com/test")); - return request; - } -} From 163314f264f45e18b91e0b7776ef825153a6cc97 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 09:13:57 -0500 Subject: [PATCH 20/30] fix: enforce min retry delay --- .../sdk/api/client/HttpRequestAttempt.java | 6 +- .../dev/openfga/sdk/util/RetryStrategy.java | 19 +- .../client/HttpRequestAttemptRetryTest.java | 306 ++++++++++++++++++ 3 files changed, 325 insertions(+), 6 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index d5be2078..303836ac 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -108,7 +108,8 @@ private CompletableFuture> attemptHttpRequest( private CompletableFuture> handleNetworkError(Throwable throwable, int retryNumber) { if (retryNumber < configuration.getMaxRetries()) { // Network errors should be retried with exponential backoff (no Retry-After header available) - Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber); + Duration retryDelay = RetryStrategy.calculateRetryDelay( + Optional.empty(), retryNumber, configuration.getMinimumRetryDelay()); // Add telemetry for network error retry addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber + 1)); @@ -125,7 +126,8 @@ private CompletableFuture> handleNetworkError(Throwable throwable private CompletableFuture> handleHttpErrorRetry( Optional retryAfterDelay, int retryNumber, FgaError error) { // Calculate appropriate delay - Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber); + Duration retryDelay = + RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber, configuration.getMinimumRetryDelay()); // Create delayed client and retry asynchronously without blocking HttpClient delayingClient = getDelayedHttpClient(retryDelay); diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 0a9b8672..00c23783 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -60,15 +60,26 @@ public static boolean shouldRetry(int statusCode) { * * @param retryAfterDelay Optional delay from Retry-After header * @param retryCount Current retry attempt (0-based) + * @param minimumRetryDelay Minimum delay to enforce (can be null) * @return Duration representing the delay before the next retry */ - public static Duration calculateRetryDelay(Optional retryAfterDelay, int retryCount) { + public static Duration calculateRetryDelay( + Optional retryAfterDelay, int retryCount, Duration minimumRetryDelay) { // If Retry-After header is present and valid, use it if (retryAfterDelay.isPresent()) { - return retryAfterDelay.get(); + Duration retryAfterValue = retryAfterDelay.get(); + // Honor minimum retry delay if configured and greater than Retry-After value + if (minimumRetryDelay != null && minimumRetryDelay.compareTo(retryAfterValue) > 0) { + return minimumRetryDelay; + } + return retryAfterValue; } - // Otherwise, use exponential backoff with jitter - return ExponentialBackoff.calculateDelay(retryCount); + // Otherwise, use exponential backoff with jitter, respecting minimum retry delay + Duration exponentialDelay = ExponentialBackoff.calculateDelay(retryCount); + if (minimumRetryDelay != null && minimumRetryDelay.compareTo(exponentialDelay) > 0) { + return minimumRetryDelay; + } + return exponentialDelay; } } diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 04f4d2ed..01ca9b2e 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -14,15 +14,19 @@ import static com.github.tomakehurst.wiremock.client.WireMock.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import com.github.tomakehurst.wiremock.WireMockServer; import com.github.tomakehurst.wiremock.core.WireMockConfiguration; import dev.openfga.sdk.api.configuration.ClientConfiguration; import dev.openfga.sdk.errors.ApiException; +import dev.openfga.sdk.errors.FgaApiInternalError; import dev.openfga.sdk.errors.FgaError; import java.net.http.HttpRequest; import java.time.Duration; +import java.time.Instant; import java.util.concurrent.ExecutionException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -419,4 +423,306 @@ void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception { assertThat(resendCount).isNotNull(); assertThat(Integer.parseInt(resendCount)).isGreaterThan(0); // Should have retry count > 0 } + + @Test + void shouldRespectGlobalMinimumRetryDelayWithExponentialBackoff() throws Exception { + // Given - Server responds with 500 errors (no Retry-After header) + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse().withStatus(500).withBody("{\"error\":\"server error\"}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Use global configuration with larger minimum retry delay + ClientConfiguration globalConfig = new ClientConfiguration() + .apiUrl("http://localhost:" + wireMockServer.port()) + .maxRetries(2) + .minimumRetryDelay(Duration.ofSeconds(2)); // Should act as floor for exponential backoff + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig); + + Instant startTime = Instant.now(); + + // When + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + assertInstanceOf(FgaApiInternalError.class, exception.getCause()); + + // Verify that it retried the expected number of times + wireMockServer.verify(1 + globalConfig.getMaxRetries(), getRequestedFor(urlEqualTo("/test"))); + + // With 2 retries and minimum 2-second delays, total time should be at least 4 seconds + assertThat(totalTime.toMillis()).isGreaterThan(3500); // Should be at least ~4 seconds + } + + @Test + void shouldRespectGlobalMinimumRetryDelayWhenRetryAfterIsSmaller() throws Exception { + // Given - Server responds with Retry-After header smaller than minimum delay + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-global-1") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "1") // 1 second + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("After-First-Request")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-global-1") + .whenScenarioStateIs("After-First-Request") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Use global configuration with larger minimum retry delay (should take precedence over Retry-After) + ClientConfiguration globalConfig = new ClientConfiguration() + .apiUrl("http://localhost:" + wireMockServer.port()) + .maxRetries(2) + .minimumRetryDelay(Duration.ofSeconds(3)); // Should override Retry-After: 1 + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig); + + Instant startTime = Instant.now(); + + // When + attempt.attemptHttpRequest().get(); // Should succeed after retry + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + // Should have respected the minimum retry delay (3 seconds) instead of Retry-After (1 second) + assertThat(totalTime.toMillis()).isGreaterThan(2800); // Should be at least ~3 seconds + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception { + // Given - Server responds with Retry-After header larger than minimum delay + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-global-2") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "2") // 2 seconds + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("After-First-Request")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-global-2") + .whenScenarioStateIs("After-First-Request") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Use global configuration with smaller minimum retry delay + ClientConfiguration globalConfig = new ClientConfiguration() + .apiUrl("http://localhost:" + wireMockServer.port()) + .maxRetries(2) + .minimumRetryDelay(Duration.ofMillis(500)); // Should NOT override Retry-After: 2 + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, globalConfig); + + Instant startTime = Instant.now(); + + // When + attempt.attemptHttpRequest().get(); // Should succeed after retry + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + // Should have respected the Retry-After header (2 seconds) over minimum delay (500ms) + // Note: Using generous bounds due to timing variability in test environments + assertThat(totalTime.toMillis()).isGreaterThan(1800); // Should be at least ~2 seconds + assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldRespectPerRequestMinimumRetryDelayOverride() throws Exception { + // Given - Server responds with 500 errors (no Retry-After header) + wireMockServer.stubFor(get(urlEqualTo("/test")) + .willReturn(aResponse().withStatus(500).withBody("{\"error\":\"server error\"}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Override with larger minimum retry delay using per-request configuration + dev.openfga.sdk.api.configuration.Configuration overriddenConfig = configuration.override( + new dev.openfga.sdk.api.configuration.ConfigurationOverride().minimumRetryDelay(Duration.ofSeconds(2))); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, overriddenConfig); + + Instant startTime = Instant.now(); + + // When + ExecutionException exception = assertThrows( + ExecutionException.class, () -> attempt.attemptHttpRequest().get()); + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + assertInstanceOf(FgaApiInternalError.class, exception.getCause()); + + // Verify that it retried the expected number of times + wireMockServer.verify(1 + overriddenConfig.getMaxRetries(), getRequestedFor(urlEqualTo("/test"))); + + // With 3 retries and minimum 2-second delays, total time should be at least 6 seconds + assertThat(totalTime.toMillis()).isGreaterThan(5500); // Should be at least ~6 seconds + } + + @Test + void shouldRespectPerRequestMaxRetriesOverride() throws Exception { + // Given - Server always responds with 500 errors + wireMockServer.stubFor(post(urlEqualTo("/stores/test-store/check")) + .willReturn(aResponse().withStatus(500).withBody("{\"error\":\"server error\"}"))); + + // Override with different max retries using per-request configuration + dev.openfga.sdk.api.configuration.ConfigurationOverride override = + new dev.openfga.sdk.api.configuration.ConfigurationOverride() + .maxRetries(5) + .minimumRetryDelay(Duration.ofMillis(10)); // Fast for testing + + // When + ExecutionException exception = assertThrows(ExecutionException.class, () -> { + // Simulate the API call with override + dev.openfga.sdk.api.configuration.Configuration effectiveConfig = configuration.override(override); + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/stores/test-store/check")) + .header("Content-Type", "application/json") + .POST(HttpRequest.BodyPublishers.ofString("{}")) + .build(); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "check", Void.class, apiClient, effectiveConfig); + attempt.attemptHttpRequest().get(); + }); + + // Then + assertInstanceOf(FgaApiInternalError.class, exception.getCause()); + + // Should have made 1 initial + 5 retries = 6 total requests + wireMockServer.verify(6, postRequestedFor(urlEqualTo("/stores/test-store/check"))); + } + + @Test + void shouldRespectPerRequestMinimumRetryDelayWhenRetryAfterIsSmaller() throws Exception { + // Given - Server responds with Retry-After header smaller than minimum delay + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-per-request-1") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "1") // 1 second + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("After-First-Request")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-per-request-1") + .whenScenarioStateIs("After-First-Request") + .willReturn(aResponse().withStatus(200).withBody(""))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Override with larger minimum retry delay (should take precedence over Retry-After) + dev.openfga.sdk.api.configuration.Configuration overriddenConfig = configuration.override( + new dev.openfga.sdk.api.configuration.ConfigurationOverride().minimumRetryDelay(Duration.ofSeconds(3))); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, overriddenConfig); + + Instant startTime = Instant.now(); + + // When + attempt.attemptHttpRequest().get(); // Should succeed after retry + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + // Should have respected the minimum retry delay (3 seconds) instead of Retry-After (1 second) + assertThat(totalTime.toMillis()).isGreaterThan(2800); // Should be at least ~3 seconds + + // Verify both requests were made + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } + + @Test + void shouldNotOverrideRetryAfterWhenItIsLargerThanMinimumDelayPerRequest() throws Exception { + // Given - Server responds with success after first retry to limit test time + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-per-request-2") + .whenScenarioStateIs("Started") + .willReturn(aResponse() + .withStatus(429) + .withHeader("Retry-After", "1") // 1 second delay + .withBody("{\"error\":\"rate limited\"}")) + .willSetStateTo("retry-attempted")); + + wireMockServer.stubFor(get(urlEqualTo("/test")) + .inScenario("retry-scenario-per-request-2") + .whenScenarioStateIs("retry-attempted") + .willReturn(aResponse().withStatus(200).withBody("{\"success\":true}"))); + + HttpRequest request = HttpRequest.newBuilder() + .uri(java.net.URI.create("http://localhost:" + wireMockServer.port() + "/test")) + .GET() + .build(); + + // Override with smaller minimum retry delay (should NOT take precedence over Retry-After) + dev.openfga.sdk.api.configuration.Configuration overriddenConfig = + configuration.override(new dev.openfga.sdk.api.configuration.ConfigurationOverride() + .minimumRetryDelay(Duration.ofMillis(500))); + + // Verify the override took effect + assertEquals(Duration.ofMillis(500), overriddenConfig.getMinimumRetryDelay()); + + HttpRequestAttempt attempt = + new HttpRequestAttempt<>(request, "test", Void.class, apiClient, overriddenConfig); + + Instant startTime = Instant.now(); + + // When - This will succeed after 1 retry + attempt.attemptHttpRequest().get(); + + Instant endTime = Instant.now(); + Duration totalTime = Duration.between(startTime, endTime); + + // Then + // Should have respected the Retry-After header (1 second) for the single retry + // Note: actual timing may vary in test environments, so we use generous bounds + assertThat(totalTime.toMillis()).isGreaterThan(800); // Should be at least ~1 second + assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive (was sometimes 4x in CI) + + // Verify initial request + 1 retry = 2 total requests + wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); + } } From f4096a35d24b7143f0ed9823e0ffabd408623d4d Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 09:51:24 -0500 Subject: [PATCH 21/30] fix: retryDelay config validation and default value --- .../sdk/api/client/HttpRequestAttempt.java | 4 -- .../sdk/api/configuration/Configuration.java | 12 ++++ .../api/configuration/ConfigurationTest.java | 69 +++++++++++++++++++ 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 303836ac..992691ae 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -200,10 +200,6 @@ private HttpClient getDelayedHttpClient(Duration retryDelay) { if (retryDelay == null || retryDelay.isZero() || retryDelay.isNegative()) { // Fallback to minimum retry delay if invalid retryDelay = configuration.getMinimumRetryDelay(); - if (retryDelay == null) { - // Default fallback if no minimum retry delay is configured - retryDelay = Duration.ofMillis(100); - } } return apiClient diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index af24c5f7..6d0a83ab 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -37,6 +37,7 @@ public class Configuration implements BaseConfiguration { private static final Duration DEFAULT_READ_TIMEOUT = Duration.ofSeconds(10); private static final Duration DEFAULT_CONNECT_TIMEOUT = Duration.ofSeconds(10); private static final int DEFAULT_MAX_RETRIES = 3; + private static final Duration DEFAULT_MINIMUM_RETRY_DELAY = Duration.ofMillis(100); private static final int MAX_ALLOWABLE_RETRIES = 15; private String apiUrl; @@ -55,6 +56,7 @@ public Configuration() { this.readTimeout = DEFAULT_READ_TIMEOUT; this.connectTimeout = DEFAULT_CONNECT_TIMEOUT; this.maxRetries = DEFAULT_MAX_RETRIES; + this.minimumRetryDelay = DEFAULT_MINIMUM_RETRY_DELAY; } /** @@ -284,7 +286,17 @@ public Integer getMaxRetries() { return maxRetries; } + /** + * Sets the minimum delay to wait before retrying a failed request. + * + * @param minimumRetryDelay The minimum delay. Must be non-negative. + * @return This Configuration instance for method chaining. + * @throws IllegalArgumentException if minimumRetryDelay is negative. + */ public Configuration minimumRetryDelay(Duration minimumRetryDelay) { + if (minimumRetryDelay != null && minimumRetryDelay.isNegative()) { + throw new IllegalArgumentException("minimumRetryDelay cannot be negative"); + } this.minimumRetryDelay = minimumRetryDelay; return this; } diff --git a/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java b/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java index e5667482..2872573b 100644 --- a/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java +++ b/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java @@ -275,4 +275,73 @@ void override_connectTimeout() { original.getConnectTimeout(), "The Configuration's default connectTimeout should be unmodified."); } + + @Test + void minimumRetryDelay_validDuration() { + // Given + Configuration config = new Configuration(); + Duration validDelay = Duration.ofMillis(500); + + // When + Configuration result = config.minimumRetryDelay(validDelay); + + // Then + assertEquals(validDelay, result.getMinimumRetryDelay()); + assertSame(config, result, "Should return the same Configuration instance for method chaining"); + } + + @Test + void minimumRetryDelay_nullValue() { + // Given + Configuration config = new Configuration(); + + // When + Configuration result = config.minimumRetryDelay(null); + + // Then + assertNull(result.getMinimumRetryDelay()); + assertSame(config, result, "Should return the same Configuration instance for method chaining"); + } + + @Test + void minimumRetryDelay_zeroDuration() { + // Given + Configuration config = new Configuration(); + Duration zeroDuration = Duration.ZERO; + + // When + Configuration result = config.minimumRetryDelay(zeroDuration); + + // Then + assertEquals(zeroDuration, result.getMinimumRetryDelay()); + assertSame(config, result, "Should return the same Configuration instance for method chaining"); + } + + @Test + void minimumRetryDelay_negativeDuration_throwsException() { + // Given + Configuration config = new Configuration(); + Duration negativeDuration = Duration.ofMillis(-100); + + // When & Then + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> config.minimumRetryDelay(negativeDuration), + "Should throw IllegalArgumentException for negative duration"); + + assertEquals("minimumRetryDelay cannot be negative", exception.getMessage()); + } + + @Test + void minimumRetryDelay_hasDefaultValue() { + // Given + Configuration config = new Configuration(); + + // When + Duration defaultDelay = config.getMinimumRetryDelay(); + + // Then + assertNotNull(defaultDelay, "minimumRetryDelay should have a default value"); + assertEquals(Duration.ofMillis(100), defaultDelay, "Default minimumRetryDelay should be 100ms"); + } } From aeecfa608a1eb324f5f20ad69b051c966b731e53 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 10:31:06 -0500 Subject: [PATCH 22/30] fix: no null minRetry value allowed --- CHANGELOG.md | 4 ++++ .../sdk/api/configuration/Configuration.java | 14 +++++++++++--- .../sdk/api/configuration/ConfigurationTest.java | 10 +++++----- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d9f37fa..aa95d99f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,13 @@ - feat: RFC 9110 compliant Retry-After header support with exponential backoff and jitter - feat: Enhanced retry strategy with delay calculation - feat: Retry-After header value exposed in error objects for better observability +- feat: Input validation for Configuration.minimumRetryDelay() to prevent negative values +- feat: Default value (100ms) now explicitly set for minimumRetryDelay configuration ### Changed - **BREAKING**: Maximum allowable retry count is now enforced at 15 (default remains 3) - **BREAKING**: FgaError now exposes Retry-After header value via getRetryAfterHeader() method +- **BREAKING**: Configuration.minimumRetryDelay() now requires non-null values and validates input, throwing IllegalArgumentException for null or negative values ### Technical Details - Implements RFC 9110 compliant Retry-After header parsing (supports both integer seconds and HTTP-date formats) @@ -21,6 +24,7 @@ **Migration Guide**: - Update error handling code if using FgaError properties - new getRetryAfterHeader() method available - Note: Maximum allowable retries is now enforced at 15 (validation added to prevent exceeding this limit) +- **IMPORTANT**: Configuration.minimumRetryDelay() now requires non-null values and validates input - ensure you're not passing null or negative Duration values, as this will now throw IllegalArgumentException. Previously null values were silently accepted and would fall back to default behavior at runtime. ## v0.8.3 diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index 6d0a83ab..0dc11193 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -289,18 +289,26 @@ public Integer getMaxRetries() { /** * Sets the minimum delay to wait before retrying a failed request. * - * @param minimumRetryDelay The minimum delay. Must be non-negative. + * @param minimumRetryDelay The minimum delay. Must be non-null and non-negative. * @return This Configuration instance for method chaining. - * @throws IllegalArgumentException if minimumRetryDelay is negative. + * @throws IllegalArgumentException if minimumRetryDelay is null or negative. */ public Configuration minimumRetryDelay(Duration minimumRetryDelay) { - if (minimumRetryDelay != null && minimumRetryDelay.isNegative()) { + if (minimumRetryDelay == null) { + throw new IllegalArgumentException("minimumRetryDelay cannot be null"); + } + if (minimumRetryDelay.isNegative()) { throw new IllegalArgumentException("minimumRetryDelay cannot be negative"); } this.minimumRetryDelay = minimumRetryDelay; return this; } + /** + * Gets the minimum delay to wait before retrying a failed request. + * + * @return The minimum retry delay. Never null. + */ @Override public Duration getMinimumRetryDelay() { return minimumRetryDelay; diff --git a/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java b/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java index 2872573b..b1d7dece 100644 --- a/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java +++ b/src/test/java/dev/openfga/sdk/api/configuration/ConfigurationTest.java @@ -295,12 +295,12 @@ void minimumRetryDelay_nullValue() { // Given Configuration config = new Configuration(); - // When - Configuration result = config.minimumRetryDelay(null); + // When & Then + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> { + config.minimumRetryDelay(null); + }); - // Then - assertNull(result.getMinimumRetryDelay()); - assertSame(config, result, "Should return the same Configuration instance for method chaining"); + assertEquals("minimumRetryDelay cannot be null", exception.getMessage()); } @Test From 2af1d07c33c39d1d0bf17bcb11b42b4a611f24d9 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 11:38:32 -0500 Subject: [PATCH 23/30] fix: larger test tolerance --- .../dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 01ca9b2e..33ce9e80 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -552,7 +552,7 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception { // Then // Should have respected the Retry-After header (2 seconds) over minimum delay (500ms) // Note: Using generous bounds due to timing variability in test environments - assertThat(totalTime.toMillis()).isGreaterThan(1800); // Should be at least ~2 seconds + assertThat(totalTime.toMillis()).isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance) assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive // Verify both requests were made From 32ac670e7091ce4993f8f0f0b34c8334502fab9a Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 11:40:25 -0500 Subject: [PATCH 24/30] fix: linter --- .../openfga/sdk/api/client/HttpRequestAttemptRetryTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 33ce9e80..09facad4 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -552,7 +552,8 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception { // Then // Should have respected the Retry-After header (2 seconds) over minimum delay (500ms) // Note: Using generous bounds due to timing variability in test environments - assertThat(totalTime.toMillis()).isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance) + assertThat(totalTime.toMillis()) + .isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance) assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive // Verify both requests were made From 4b7215f4d95c564708f6991ca953a2001c25e93a Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 12:03:56 -0500 Subject: [PATCH 25/30] test debugging output --- .../java/dev/openfga/sdk/api/client/HttpRequestAttempt.java | 2 +- .../dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java index 992691ae..fd3d03ac 100644 --- a/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java +++ b/src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java @@ -145,7 +145,7 @@ private CompletableFuture> processHttpResponse( if (retryNumber < configuration.getMaxRetries()) { // Parse Retry-After header if present Optional retryAfterDelay = - response.headers().firstValue("retry-after").flatMap(RetryAfterHeaderParser::parseRetryAfter); + response.headers().firstValue("Retry-After").flatMap(RetryAfterHeaderParser::parseRetryAfter); // Check if we should retry based on the new strategy if (RetryStrategy.shouldRetry(statusCode)) { diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 09facad4..65dec0f3 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -552,6 +552,8 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception { // Then // Should have respected the Retry-After header (2 seconds) over minimum delay (500ms) // Note: Using generous bounds due to timing variability in test environments + System.out.println("Actual retry duration: " + totalTime.toMillis() + " ms"); + assertThat(totalTime.toMillis()) .isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance) assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive From f04d224561baec1c8ebd04d9d5ecbc82506a716c Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 5 Aug 2025 12:14:56 -0500 Subject: [PATCH 26/30] increased test tolerance --- .../dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 65dec0f3..1e7d1c66 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -556,7 +556,7 @@ void shouldUseRetryAfterWhenLargerThanGlobalMinimumDelay() throws Exception { assertThat(totalTime.toMillis()) .isGreaterThan(1200); // Should be at least ~2 seconds (with larger CI tolerance) - assertThat(totalTime.toMillis()).isLessThan(10000); // But not excessive + assertThat(totalTime.toMillis()).isLessThan(15000); // But not excessive // Verify both requests were made wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); From 57bfd190cf5fca3aa71daf94c758b16baf870298 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 6 Aug 2025 14:37:31 -0500 Subject: [PATCH 27/30] minimum retry delay config should be used as the base delay --- .../sdk/api/configuration/Configuration.java | 7 +- .../openfga/sdk/util/ExponentialBackoff.java | 31 +++-- .../dev/openfga/sdk/util/RetryStrategy.java | 12 +- .../client/HttpRequestAttemptRetryTest.java | 5 +- .../sdk/util/ExponentialBackoffTest.java | 63 +++++++-- .../openfga/sdk/util/RetryStrategyTest.java | 123 ++++++++++++++++++ 6 files changed, 206 insertions(+), 35 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index 0dc11193..3bf8287e 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -37,9 +37,14 @@ public class Configuration implements BaseConfiguration { private static final Duration DEFAULT_READ_TIMEOUT = Duration.ofSeconds(10); private static final Duration DEFAULT_CONNECT_TIMEOUT = Duration.ofSeconds(10); private static final int DEFAULT_MAX_RETRIES = 3; - private static final Duration DEFAULT_MINIMUM_RETRY_DELAY = Duration.ofMillis(100); private static final int MAX_ALLOWABLE_RETRIES = 15; + /** + * Default minimum retry delay of 100ms. + * This value is also used as the default base delay for exponential backoff calculations. + */ + public static final Duration DEFAULT_MINIMUM_RETRY_DELAY = Duration.ofMillis(100); + private String apiUrl; private Credentials credentials; private String userAgent; diff --git a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java index ffd256f4..4439e5ff 100644 --- a/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java +++ b/src/main/java/dev/openfga/sdk/util/ExponentialBackoff.java @@ -19,13 +19,12 @@ * Utility class for calculating exponential backoff delays with jitter. * * Implements the retry strategy specified in GitHub issue #155: - * - Base delay: 2^retryCount * 100ms + * - Base delay: 2^retryCount * baseDelay (configurable) * - Jitter: Random value between base and 2 * base * - Maximum delay: 120 seconds (capped after 10th retry) */ public class ExponentialBackoff { - private static final int BASE_DELAY_MS = 100; private static final int MAX_DELAY_SECONDS = 120; private static final Random RANDOM = new Random(); @@ -37,10 +36,11 @@ private ExponentialBackoff() { * Calculates the exponential backoff delay with jitter for a given retry attempt. * * @param retryCount The current retry attempt (0-based, so first retry is 0) + * @param baseDelay The base delay to use for exponential calculation * @return Duration representing the delay before the next retry */ - public static Duration calculateDelay(int retryCount) { - return calculateDelay(retryCount, RANDOM); + public static Duration calculateDelay(int retryCount, Duration baseDelay) { + return calculateDelay(retryCount, baseDelay, RANDOM); } /** @@ -48,26 +48,33 @@ public static Duration calculateDelay(int retryCount) { * This method is primarily for testing purposes to ensure deterministic behavior. * * @param retryCount The current retry attempt (0-based, so first retry is 0) + * @param baseDelay The base delay to use for exponential calculation * @param random Random instance to use for jitter calculation * @return Duration representing the delay before the next retry */ - static Duration calculateDelay(int retryCount, Random random) { + static Duration calculateDelay(int retryCount, Duration baseDelay, Random random) { if (retryCount < 0) { return Duration.ZERO; } - // Calculate base delay: 2^retryCount * 100ms - long baseDelayMs = (long) Math.pow(2, retryCount) * BASE_DELAY_MS; + // Use provided base delay (caller must provide a valid value) + if (baseDelay == null) { + throw new IllegalArgumentException("baseDelay cannot be null"); + } + long baseDelayMs = baseDelay.toMillis(); + + // Calculate exponential delay: 2^retryCount * baseDelay + long exponentialDelayMs = (long) Math.pow(2, retryCount) * baseDelayMs; // Cap at maximum delay long maxDelayMs = MAX_DELAY_SECONDS * 1000L; - if (baseDelayMs > maxDelayMs) { - baseDelayMs = maxDelayMs; + if (exponentialDelayMs > maxDelayMs) { + exponentialDelayMs = maxDelayMs; } - // Add jitter: random value between baseDelay and 2 * baseDelay - long minDelayMs = baseDelayMs; - long maxDelayMsWithJitter = Math.min(baseDelayMs * 2, maxDelayMs); + // Add jitter: random value between exponentialDelay and 2 * exponentialDelay + long minDelayMs = exponentialDelayMs; + long maxDelayMsWithJitter = Math.min(exponentialDelayMs * 2, maxDelayMs); // Generate random delay within the jitter range long jitterRange = maxDelayMsWithJitter - minDelayMs; diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 00c23783..17910159 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -12,6 +12,7 @@ package dev.openfga.sdk.util; +import dev.openfga.sdk.api.configuration.Configuration; import dev.openfga.sdk.errors.HttpStatusCode; import java.time.Duration; import java.util.Optional; @@ -60,7 +61,7 @@ public static boolean shouldRetry(int statusCode) { * * @param retryAfterDelay Optional delay from Retry-After header * @param retryCount Current retry attempt (0-based) - * @param minimumRetryDelay Minimum delay to enforce (can be null) + * @param minimumRetryDelay Minimum delay to enforce (also used as base delay for exponential backoff) * @return Duration representing the delay before the next retry */ public static Duration calculateRetryDelay( @@ -75,11 +76,8 @@ public static Duration calculateRetryDelay( return retryAfterValue; } - // Otherwise, use exponential backoff with jitter, respecting minimum retry delay - Duration exponentialDelay = ExponentialBackoff.calculateDelay(retryCount); - if (minimumRetryDelay != null && minimumRetryDelay.compareTo(exponentialDelay) > 0) { - return minimumRetryDelay; - } - return exponentialDelay; + // Otherwise, use exponential backoff with jitter, respecting minimum retry delay + Duration baseDelay = minimumRetryDelay != null ? minimumRetryDelay : Configuration.DEFAULT_MINIMUM_RETRY_DELAY; + return ExponentialBackoff.calculateDelay(retryCount, baseDelay); } } diff --git a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java index 1e7d1c66..8ada4afa 100644 --- a/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java +++ b/src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java @@ -283,9 +283,10 @@ void shouldUseExponentialBackoffWhenNoRetryAfterHeader() throws Exception { // Verify both requests were made wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test"))); - // Verify some delay occurred (exponential backoff should add at least 100ms for first retry) + // Verify some delay occurred (exponential backoff with 10ms minimum delay) // Note: Using a generous range due to test timing variability - assertThat(endTime - startTime).isGreaterThan(400L); + // With minimumRetryDelay=10ms, first retry should be at least 10ms + assertThat(endTime - startTime).isGreaterThan(8L); } @Test diff --git a/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java index ad923612..d26ccf22 100644 --- a/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java +++ b/src/test/java/dev/openfga/sdk/util/ExponentialBackoffTest.java @@ -13,6 +13,7 @@ package dev.openfga.sdk.util; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.Duration; import java.util.Random; @@ -24,10 +25,11 @@ class ExponentialBackoffTest { void calculateDelay_withRetryCountZero_shouldReturnBaseDelay() { // Given int retryCount = 0; + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // Fixed seed for deterministic testing // When - Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); // Then // For retry count 0: 2^0 * 100ms = 100ms base @@ -39,10 +41,11 @@ void calculateDelay_withRetryCountZero_shouldReturnBaseDelay() { void calculateDelay_withRetryCountOne_shouldReturnDoubledDelay() { // Given int retryCount = 1; + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // Fixed seed for deterministic testing // When - Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); // Then // For retry count 1: 2^1 * 100ms = 200ms base @@ -54,10 +57,11 @@ void calculateDelay_withRetryCountOne_shouldReturnDoubledDelay() { void calculateDelay_withRetryCountTwo_shouldReturnQuadrupledDelay() { // Given int retryCount = 2; + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // Fixed seed for deterministic testing // When - Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); // Then // For retry count 2: 2^2 * 100ms = 400ms base @@ -69,10 +73,11 @@ void calculateDelay_withRetryCountTwo_shouldReturnQuadrupledDelay() { void calculateDelay_withHighRetryCount_shouldCapAtMaximum() { // Given int retryCount = 10; // This would normally result in 2^10 * 100ms = 102400ms + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // Fixed seed for deterministic testing // When - Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); // Then // Should be capped at 120 seconds (120000ms) @@ -85,7 +90,7 @@ void calculateDelay_withNegativeRetryCount_shouldReturnZero() { int retryCount = -1; // When - Duration result = ExponentialBackoff.calculateDelay(retryCount); + Duration result = ExponentialBackoff.calculateDelay(retryCount, Duration.ofMillis(100)); // Then assertThat(result).isEqualTo(Duration.ZERO); @@ -97,7 +102,7 @@ void calculateDelay_withoutRandomParameter_shouldReturnValidRange() { int retryCount = 1; // When - Duration result = ExponentialBackoff.calculateDelay(retryCount); + Duration result = ExponentialBackoff.calculateDelay(retryCount, Duration.ofMillis(100)); // Then // For retry count 1: 2^1 * 100ms = 200ms base @@ -111,9 +116,9 @@ void calculateDelay_shouldProduceVariousResults() { int retryCount = 1; // When - call multiple times to ensure randomness - Duration result1 = ExponentialBackoff.calculateDelay(retryCount); - Duration result2 = ExponentialBackoff.calculateDelay(retryCount); - Duration result3 = ExponentialBackoff.calculateDelay(retryCount); + Duration result1 = ExponentialBackoff.calculateDelay(retryCount, Duration.ofMillis(100)); + Duration result2 = ExponentialBackoff.calculateDelay(retryCount, Duration.ofMillis(100)); + Duration result3 = ExponentialBackoff.calculateDelay(retryCount, Duration.ofMillis(100)); // Then - all should be in valid range but likely different assertThat(result1.toMillis()).isBetween(200L, 400L); @@ -125,12 +130,13 @@ void calculateDelay_shouldProduceVariousResults() { void calculateDelay_withFixedRandom_shouldBeDeterministic() { // Given int retryCount = 1; + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom1 = new Random(123); Random fixedRandom2 = new Random(123); // Same seed // When - Duration result1 = ExponentialBackoff.calculateDelay(retryCount, fixedRandom1); - Duration result2 = ExponentialBackoff.calculateDelay(retryCount, fixedRandom2); + Duration result1 = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom1); + Duration result2 = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom2); // Then assertThat(result1).isEqualTo(result2); @@ -139,13 +145,14 @@ void calculateDelay_withFixedRandom_shouldBeDeterministic() { @Test void calculateDelay_progressionTest_shouldFollowExponentialPattern() { // Given + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // When & Then - test the progression follows expected pattern for (int i = 0; i < 8; i++) { // Reset the random seed for consistent results across iterations fixedRandom.setSeed(42); - Duration delay = ExponentialBackoff.calculateDelay(i, fixedRandom); + Duration delay = ExponentialBackoff.calculateDelay(i, baseDelay, fixedRandom); long expectedBaseMs = (long) Math.pow(2, i) * 100; long expectedMaxMs = Math.min(expectedBaseMs * 2, 120000); @@ -160,13 +167,43 @@ void calculateDelay_progressionTest_shouldFollowExponentialPattern() { void calculateDelay_atCapThreshold_shouldCapCorrectly() { // Given - retry count that would exceed 120s base delay int retryCount = 11; // 2^11 * 100ms = 204800ms > 120000ms + Duration baseDelay = Duration.ofMillis(100); Random fixedRandom = new Random(42); // When - Duration result = ExponentialBackoff.calculateDelay(retryCount, fixedRandom); + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); // Then - should be capped at 120s maximum assertThat(result.toMillis()).isLessThanOrEqualTo(120000L); assertThat(result.toMillis()).isGreaterThanOrEqualTo(120000L); // Should be exactly at cap for base delay } + + @Test + void calculateDelay_withCustomBaseDelay_shouldUseConfigurableBase() { + // Given - custom base delay of 500ms + int retryCount = 1; + Duration baseDelay = Duration.ofMillis(500); + Random fixedRandom = new Random(42); + + // When + Duration result = ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom); + + // Then + // For retry count 1 with 500ms base: 2^1 * 500ms = 1000ms base + // With jitter: between 1000ms and 2000ms + assertThat(result.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateDelay_withNullBaseDelay_shouldThrowException() { + // Given + int retryCount = 0; + Duration baseDelay = null; + Random fixedRandom = new Random(42); + + // When & Then + assertThatThrownBy(() -> ExponentialBackoff.calculateDelay(retryCount, baseDelay, fixedRandom)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("baseDelay cannot be null"); + } } diff --git a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java index e69de29b..b5ca6edf 100644 --- a/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java +++ b/src/test/java/dev/openfga/sdk/util/RetryStrategyTest.java @@ -0,0 +1,123 @@ +/* + * OpenFGA + * A high performance and flexible authorization/permission engine built for developers and inspired by Google Zanzibar. + * + * The version of the OpenAPI document: 1.x + * Contact: community@openfga.dev + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + +package dev.openfga.sdk.util; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class RetryStrategyTest { + + @Test + void calculateRetryDelay_withRetryAfterHeader_shouldUseRetryAfterValue() { + // Given + Optional retryAfterDelay = Optional.of(Duration.ofMillis(5000)); + int retryCount = 1; + Duration minimumRetryDelay = Duration.ofMillis(100); + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); + + // Then + assertThat(result).isEqualTo(Duration.ofMillis(5000)); + } + + @Test + void calculateRetryDelay_withRetryAfterSmallerThanMinimum_shouldUseMinimum() { + // Given + Optional retryAfterDelay = Optional.of(Duration.ofMillis(50)); + int retryCount = 1; + Duration minimumRetryDelay = Duration.ofMillis(200); + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); + + // Then + assertThat(result).isEqualTo(Duration.ofMillis(200)); + } + + @Test + void calculateRetryDelay_withoutRetryAfter_shouldUseMinimumAsBaseDelay() { + // Given + Optional retryAfterDelay = Optional.empty(); + int retryCount = 1; + Duration minimumRetryDelay = Duration.ofMillis(500); + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); + + // Then + // For retry count 1 with 500ms base: 2^1 * 500ms = 1000ms base + // With jitter: between 1000ms and 2000ms + assertThat(result.toMillis()).isBetween(1000L, 2000L); + } + + @Test + void calculateRetryDelay_withNullMinimum_shouldUseDefaultBase() { + // Given + Optional retryAfterDelay = Optional.empty(); + int retryCount = 1; + Duration minimumRetryDelay = null; + + // When + Duration result = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryCount, minimumRetryDelay); + + // Then + // Should use default 100ms base delay + // For retry count 1: 2^1 * 100ms = 200ms base + // With jitter: between 200ms and 400ms + assertThat(result.toMillis()).isBetween(200L, 400L); + } + + @Test + void shouldRetry_with429_shouldReturnTrue() { + assertThat(RetryStrategy.shouldRetry(429)).isTrue(); + } + + @Test + void shouldRetry_with500_shouldReturnTrue() { + assertThat(RetryStrategy.shouldRetry(500)).isTrue(); + } + + @Test + void shouldRetry_with502_shouldReturnTrue() { + assertThat(RetryStrategy.shouldRetry(502)).isTrue(); + } + + @Test + void shouldRetry_with503_shouldReturnTrue() { + assertThat(RetryStrategy.shouldRetry(503)).isTrue(); + } + + @Test + void shouldRetry_with504_shouldReturnTrue() { + assertThat(RetryStrategy.shouldRetry(504)).isTrue(); + } + + @Test + void shouldRetry_with400_shouldReturnFalse() { + assertThat(RetryStrategy.shouldRetry(400)).isFalse(); + } + + @Test + void shouldRetry_with404_shouldReturnFalse() { + assertThat(RetryStrategy.shouldRetry(404)).isFalse(); + } + + @Test + void shouldRetry_with501_shouldReturnFalse() { + assertThat(RetryStrategy.shouldRetry(501)).isFalse(); + } +} From c242df1aa354b9356bfc10467cb579460edd33c1 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 7 Aug 2025 09:11:55 -0500 Subject: [PATCH 28/30] lint fix --- src/main/java/dev/openfga/sdk/util/RetryStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 17910159..14af95e3 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -76,7 +76,7 @@ public static Duration calculateRetryDelay( return retryAfterValue; } - // Otherwise, use exponential backoff with jitter, respecting minimum retry delay + // Otherwise, use exponential backoff with jitter, respecting minimum retry delay Duration baseDelay = minimumRetryDelay != null ? minimumRetryDelay : Configuration.DEFAULT_MINIMUM_RETRY_DELAY; return ExponentialBackoff.calculateDelay(retryCount, baseDelay); } From e4b0e9768303f303135e7d9814de1f2cb0df0cf1 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 7 Aug 2025 09:13:12 -0500 Subject: [PATCH 29/30] Update src/main/java/dev/openfga/sdk/util/RetryStrategy.java Co-authored-by: Raghd Hamzeh --- src/main/java/dev/openfga/sdk/util/RetryStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java index 14af95e3..921f2e4a 100644 --- a/src/main/java/dev/openfga/sdk/util/RetryStrategy.java +++ b/src/main/java/dev/openfga/sdk/util/RetryStrategy.java @@ -61,7 +61,7 @@ public static boolean shouldRetry(int statusCode) { * * @param retryAfterDelay Optional delay from Retry-After header * @param retryCount Current retry attempt (0-based) - * @param minimumRetryDelay Minimum delay to enforce (also used as base delay for exponential backoff) + * @param minimumRetryDelay Base delay for exponential backoff * @return Duration representing the delay before the next retry */ public static Duration calculateRetryDelay( From 03062c2e94eb275ad5b2321ec3d1744b78692ee3 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 7 Aug 2025 09:13:27 -0500 Subject: [PATCH 30/30] Update src/main/java/dev/openfga/sdk/api/configuration/Configuration.java Co-authored-by: Raghd Hamzeh --- .../java/dev/openfga/sdk/api/configuration/Configuration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java index 3bf8287e..fd8c894c 100644 --- a/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java +++ b/src/main/java/dev/openfga/sdk/api/configuration/Configuration.java @@ -41,7 +41,7 @@ public class Configuration implements BaseConfiguration { /** * Default minimum retry delay of 100ms. - * This value is also used as the default base delay for exponential backoff calculations. + * This value is used as the default base delay for exponential backoff calculations. */ public static final Duration DEFAULT_MINIMUM_RETRY_DELAY = Duration.ofMillis(100);