Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #4988: ensuring the previous response is closed #4990

Merged
merged 1 commit into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Fix #4793: (java-generator) Fix broken POJO generation when two schema properties collide into a single field name
* Fix #4963: Openshift Client return 403 when use websocket
* Fix #4985: triggering the immediate cleanup of the okhttp idle task
* Fix #4988: Ensuring that previous requests are closed before retry
* fix #5002: Jetty response completion accounts for header processing
* Fix #5009: addressing issue with serialization of wrapped polymophic types

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ protected <V> void retryWithExponentialBackoff(CompletableFuture<V> result,
LOG.debug("HTTP operation on url: {} should be retried as the response code was {}, retrying after {} millis",
uri, code, retryInterval);
retry = true;
cancel.accept(response);
}
} else if (throwable instanceof IOException) {
LOG.debug(String.format("HTTP operation on url: %s should be retried after %d millis because of IOException",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ void testHttpRetryWithLessFailuresThanRetries() throws Exception {
.withRequestRetryBackoffInterval(50).build())
.build();

final HttpResponse<AsyncBody> error = new TestHttpResponse<AsyncBody>().withCode(500);
final HttpResponse<AsyncBody> error = new TestHttpResponse<AsyncBody>().withBody(Mockito.mock(AsyncBody.class))
.withCode(500);
client.getRespFutures().add(CompletableFuture.completedFuture(error));
client.getRespFutures().add(CompletableFuture.completedFuture(error));
client.getRespFutures().add(CompletableFuture.completedFuture(error));
Expand All @@ -155,7 +156,7 @@ void testHttpRetryWithLessFailuresThanRetries() throws Exception {
});

// should ultimately succeed with the final 200
assertEquals(200, consumeFuture.get().code());
assertEquals(200, consumeFuture.get(2, TimeUnit.MINUTES).code());

// only 4 requests issued
assertEquals(4, client.getRespFutures().size());
Expand All @@ -179,9 +180,35 @@ void testWebSocketWithLessFailuresThanRetries() throws Exception {
client.getWsFutures().add(client.getWsFutures().get(0));
client.getWsFutures().add(CompletableFuture.completedFuture((new WebSocketResponse(ws, null))));

future.get();
future.get(2, TimeUnit.MINUTES);

assertEquals(3, client.getWsFutures().size());
}

@Test
void testClosePreviousBeforeRetry() throws Exception {
client = client.newBuilder().tag(new RequestConfigBuilder()
.withRequestRetryBackoffLimit(1)
.withRequestRetryBackoffInterval(50).build())
.build();

final HttpResponse<AsyncBody> error = new TestHttpResponse<AsyncBody>().withBody(Mockito.mock(AsyncBody.class))
.withCode(503);
client.getRespFutures().add(CompletableFuture.completedFuture(error));
client.getRespFutures().add(CompletableFuture.completedFuture(new TestHttpResponse<AsyncBody>().withCode(200)));

CompletableFuture<HttpResponse<AsyncBody>> consumeFuture = client.consumeBytes(
client.newHttpRequestBuilder().uri("http://localhost").build(),
(value, asyncBody) -> {
});

Mockito.verify(error.body()).cancel();

// should ultimately succeed with the final 200
assertEquals(200, consumeFuture.get(2, TimeUnit.MINUTES).code());

// only 2 requests issued
assertEquals(2, client.getRespFutures().size());
}

}