Skip to content

Commit

Permalink
Wrap synchronous http call with timeout in RequestRetryPolicy (#38724)
Browse files Browse the repository at this point in the history
  • Loading branch information
dennyac authored Feb 29, 2024
1 parent 5499ab5 commit 7f736fb
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 9 deletions.
1 change: 1 addition & 0 deletions sdk/storage/azure-storage-common/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

### Bugs Fixed
- Fixed bug where RequestRetryOptions.tryTimeout adds delay to the client request in the synchronous http client flow.

### Other Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,19 +253,17 @@ private HttpResponse attemptSync(final HttpPipelineCallContext context, HttpPipe
* We want to send the request with a given timeout, but we don't want to kickoff that timeout-bound
* operation until after the retry backoff delay, so we call delaySubscription.
*/
HttpResponse response = next.clone().processSync();
Mono<HttpResponse> httpResponseMono = Mono.fromCallable(() -> next.clone().processSync());

// Default try timeout is Integer.MAX_VALUE seconds, if it's that don't set a timeout as that's about 68 years
// and would likely never complete.
// TODO (alzimmer): Think about not adding this if it's over a certain length, like 1 year.
if (this.requestRetryOptions.getTryTimeoutDuration().getSeconds() != Integer.MAX_VALUE) {
try {
Thread.sleep(this.requestRetryOptions.getTryTimeoutDuration().toMillis());
} catch (InterruptedException ie) {
throw LOGGER.logExceptionAsError(new RuntimeException(ie));
}
httpResponseMono = httpResponseMono.timeout(this.requestRetryOptions.getTryTimeoutDuration());
}

HttpResponse response = httpResponseMono.block();

boolean newConsiderSecondary = considerSecondary;
int statusCode = response.getStatusCode();
boolean retry = shouldStatusCodeBeRetried(statusCode, tryingPrimary);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ private void beforeSendingRequest(HttpRequest request) {

@Override
public HttpResponse sendSync(HttpRequest request, Context context) {
beforeSendingRequest(request);
return response;
return send(request).block();
}

@Override
public Mono<HttpResponse> send(HttpRequest request) {
beforeSendingRequest(request);
return Mono.just(response);
return count < 3
? Mono.just(response).delaySubscription(Duration.ofSeconds(5)) : Mono.just(response);
}
})
.policies(new RequestRetryPolicy(retryTestOptions))
Expand Down

0 comments on commit 7f736fb

Please sign in to comment.