Skip to content

Commit a7ae65e

Browse files
jimmyjamesjimmyjames
committed
handle network errors, cleanup
1 parent 1b43da5 commit a7ae65e

3 files changed

Lines changed: 77 additions & 63 deletions

File tree

‎src/main/java/dev/openfga/sdk/api/client/HttpRequestAttempt.java‎

Lines changed: 62 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -97,79 +97,89 @@ private CompletableFuture<ApiResponse<T>> attemptHttpRequest(
9797
.handle((response, throwable) -> {
9898
if (throwable != null) {
9999
// Handle network errors (no HTTP response received)
100-
if (retryNumber < configuration.getMaxRetries()) {
101-
// Network errors should be retried with exponential backoff (no Retry-After header available)
102-
Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber);
103-
HttpClient delayingClient = getDelayedHttpClient(retryDelay);
104-
return attemptHttpRequest(delayingClient, retryNumber + 1, throwable);
105-
} else {
106-
// Max retries exceeded, fail with the network error
107-
return CompletableFuture.<ApiResponse<T>>failedFuture(new ApiException(throwable));
108-
}
100+
return handleNetworkError(throwable, retryNumber);
109101
}
110102
// No network error, proceed with normal HTTP response handling
111103
return processHttpResponse(response, retryNumber, previousError);
112104
})
113105
.thenCompose(future -> future);
114106
}
115107

116-
private CompletableFuture<ApiResponse<T>> processHttpResponse(
117-
HttpResponse<String> response, int retryNumber, Throwable previousError) {
118-
return CompletableFuture.completedFuture(response).thenCompose(httpResponse -> {
119-
Optional<FgaError> fgaError =
120-
FgaError.getError(name, request, configuration, response, previousError);
108+
private CompletableFuture<ApiResponse<T>> handleNetworkError(Throwable throwable, int retryNumber) {
109+
if (retryNumber < configuration.getMaxRetries()) {
110+
// Network errors should be retried with exponential backoff (no Retry-After header available)
111+
Duration retryDelay = RetryStrategy.calculateRetryDelay(Optional.empty(), retryNumber);
121112

122-
if (fgaError.isPresent()) {
123-
FgaError error = fgaError.get();
124-
int statusCode = error.getStatusCode();
113+
// Add telemetry for network error retry
114+
addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber + 1));
125115

126-
if (retryNumber < configuration.getMaxRetries()) {
127-
// Parse Retry-After header if present
128-
Optional<Duration> retryAfterDelay = response.headers()
129-
.firstValue("retry-after")
130-
.flatMap(RetryAfterHeaderParser::parseRetryAfter);
116+
// Create delayed client and retry asynchronously without blocking
117+
HttpClient delayingClient = getDelayedHttpClient(retryDelay);
118+
return attemptHttpRequest(delayingClient, retryNumber + 1, throwable);
119+
} else {
120+
// Max retries exceeded, fail with the network error
121+
return CompletableFuture.failedFuture(new ApiException(throwable));
122+
}
123+
}
131124

132-
boolean hasValidRetryAfter = retryAfterDelay.isPresent();
125+
private CompletableFuture<ApiResponse<T>> handleHttpErrorRetry(
126+
Optional<Duration> retryAfterDelay, int retryNumber, FgaError error) {
127+
// Calculate appropriate delay
128+
Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber);
133129

134-
// Check if we should retry based on the new strategy
135-
if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) {
136-
// Calculate appropriate delay
137-
Duration retryDelay = RetryStrategy.calculateRetryDelay(retryAfterDelay, retryNumber);
130+
// Create delayed client and retry asynchronously without blocking
131+
HttpClient delayingClient = getDelayedHttpClient(retryDelay);
132+
return attemptHttpRequest(delayingClient, retryNumber + 1, error);
133+
}
138134

139-
HttpClient delayingClient = getDelayedHttpClient(retryDelay);
135+
private CompletableFuture<ApiResponse<T>> processHttpResponse(
136+
HttpResponse<String> response, int retryNumber, Throwable previousError) {
137+
Optional<FgaError> fgaError = FgaError.getError(name, request, configuration, response, previousError);
140138

141-
return attemptHttpRequest(delayingClient, retryNumber + 1, error);
142-
}
143-
}
139+
if (fgaError.isPresent()) {
140+
FgaError error = fgaError.get();
141+
int statusCode = error.getStatusCode();
144142

145-
return CompletableFuture.failedFuture(error);
146-
}
143+
if (retryNumber < configuration.getMaxRetries()) {
144+
// Parse Retry-After header if present
145+
Optional<Duration> retryAfterDelay =
146+
response.headers().firstValue("retry-after").flatMap(RetryAfterHeaderParser::parseRetryAfter);
147147

148-
addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials()));
148+
boolean hasValidRetryAfter = retryAfterDelay.isPresent();
149149

150-
if (retryNumber > 0) {
151-
addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber));
152-
}
150+
// Check if we should retry based on the new strategy
151+
if (RetryStrategy.shouldRetry(request, statusCode, hasValidRetryAfter)) {
152+
return handleHttpErrorRetry(retryAfterDelay, retryNumber, error);
153+
} else {
154+
}
155+
}
153156

154-
if (response.headers().firstValue("fga-query-duration-ms").isPresent()) {
155-
String queryDuration = response.headers()
156-
.firstValue("fga-query-duration-ms")
157-
.orElse(null);
157+
return CompletableFuture.failedFuture(error);
158+
}
158159

159-
if (!isNullOrWhitespace(queryDuration)) {
160-
double queryDurationDouble = Double.parseDouble(queryDuration);
161-
telemetry.metrics().queryDuration(queryDurationDouble, this.getTelemetryAttributes());
162-
}
163-
}
160+
addTelemetryAttributes(Attributes.fromHttpResponse(response, this.configuration.getCredentials()));
161+
162+
if (retryNumber > 0) {
163+
addTelemetryAttribute(Attributes.HTTP_REQUEST_RESEND_COUNT, String.valueOf(retryNumber));
164+
}
165+
166+
if (response.headers().firstValue("fga-query-duration-ms").isPresent()) {
167+
String queryDuration =
168+
response.headers().firstValue("fga-query-duration-ms").orElse(null);
169+
170+
if (!isNullOrWhitespace(queryDuration)) {
171+
double queryDurationDouble = Double.parseDouble(queryDuration);
172+
telemetry.metrics().queryDuration(queryDurationDouble, this.getTelemetryAttributes());
173+
}
174+
}
164175

165-
Double requestDuration = (double) (System.currentTimeMillis() - requestStarted);
176+
Double requestDuration = (double) (System.currentTimeMillis() - requestStarted);
166177

167-
telemetry.metrics().requestDuration(requestDuration, this.getTelemetryAttributes());
178+
telemetry.metrics().requestDuration(requestDuration, this.getTelemetryAttributes());
168179

169-
return deserializeResponse(response)
170-
.thenApply(modeledResponse -> new ApiResponse<>(
171-
response.statusCode(), response.headers().map(), response.body(), modeledResponse));
172-
});
180+
return deserializeResponse(response)
181+
.thenApply(modeledResponse -> new ApiResponse<>(
182+
response.statusCode(), response.headers().map(), response.body(), modeledResponse));
173183
}
174184

175185
private CompletableFuture<T> deserializeResponse(HttpResponse<String> response) {

‎src/main/java/dev/openfga/sdk/util/RetryStrategy.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import java.net.http.HttpRequest;
1717
import java.time.Duration;
1818
import java.util.Optional;
19-
import java.util.Set;
2019

2120
/**
2221
* Utility class for determining retry behavior based on HTTP status codes.
@@ -75,6 +74,4 @@ public static Duration calculateRetryDelay(Optional<Duration> retryAfterDelay, i
7574
// Otherwise, use exponential backoff with jitter
7675
return ExponentialBackoff.calculateDelay(retryCount);
7776
}
78-
79-
8077
}

‎src/test/java/dev/openfga/sdk/api/client/HttpRequestAttemptRetryTest.java‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,13 @@ void shouldRetryOnConnectionTimeout() throws Exception {
324324
// Given - Capture port before stopping server
325325
int serverPort = wireMockServer.port();
326326
wireMockServer.stop();
327-
327+
328328
// Create configuration with shorter timeout for faster test
329329
ClientConfiguration timeoutConfig = new ClientConfiguration()
330330
.apiUrl("http://localhost:" + serverPort)
331331
.maxRetries(2)
332332
.minimumRetryDelay(Duration.ofMillis(10));
333-
333+
334334
HttpRequest request = HttpRequest.newBuilder()
335335
.uri(java.net.URI.create("http://localhost:" + serverPort + "/test"))
336336
.GET()
@@ -357,15 +357,14 @@ void shouldRetryOnUnknownHost() throws Exception {
357357
.apiUrl("http://invalid-hostname-that-does-not-exist.local")
358358
.maxRetries(2)
359359
.minimumRetryDelay(Duration.ofMillis(10));
360-
360+
361361
HttpRequest request = HttpRequest.newBuilder()
362362
.uri(java.net.URI.create("http://invalid-hostname-that-does-not-exist.local/test"))
363363
.GET()
364364
.timeout(Duration.ofMillis(1000))
365365
.build();
366366

367-
HttpRequestAttempt<Void> attempt =
368-
new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig);
367+
HttpRequestAttempt<Void> attempt = new HttpRequestAttempt<>(request, "test", Void.class, apiClient, dnsConfig);
369368

370369
// When & Then
371370
ExecutionException exception = assertThrows(
@@ -382,14 +381,14 @@ void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception {
382381
// Given - Capture port before stopping server
383382
int serverPort = wireMockServer.port();
384383
wireMockServer.stop();
385-
384+
386385
long startTime = System.currentTimeMillis();
387-
386+
388387
ClientConfiguration networkConfig = new ClientConfiguration()
389388
.apiUrl("http://localhost:" + serverPort)
390389
.maxRetries(3)
391390
.minimumRetryDelay(Duration.ofMillis(50)); // Longer delay to measure timing
392-
391+
393392
HttpRequest request = HttpRequest.newBuilder()
394393
.uri(java.net.URI.create("http://localhost:" + serverPort + "/test"))
395394
.GET()
@@ -411,5 +410,13 @@ void shouldRetryNetworkErrorsWithExponentialBackoff() throws Exception {
411410
assertThat(exception.getCause()).isInstanceOf(ApiException.class);
412411
ApiException apiException = (ApiException) exception.getCause();
413412
assertThat(apiException.getCause()).isNotNull(); // Should have underlying network error
413+
414+
// Verify telemetry attributes were set for network error retries
415+
assertThat(attempt.getTelemetryAttributes())
416+
.containsKey(dev.openfga.sdk.telemetry.Attributes.HTTP_REQUEST_RESEND_COUNT);
417+
String resendCount =
418+
attempt.getTelemetryAttributes().get(dev.openfga.sdk.telemetry.Attributes.HTTP_REQUEST_RESEND_COUNT);
419+
assertThat(resendCount).isNotNull();
420+
assertThat(Integer.parseInt(resendCount)).isGreaterThan(0); // Should have retry count > 0
414421
}
415422
}

0 commit comments

Comments
 (0)