Skip to content

Commit

Permalink
fix fabric8io#4988: ensuring the previous response is closed
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Apr 5, 2023
1 parent b44cf33 commit 6fc8407
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#### Bugs
* Fix #4963: Openshift Client return 403 when use websocket
* Fix #4988: Ensuring that previous requests are closed before retry

#### Improvements
* Fix #4477 exposing LeaderElector.release to force an elector to give up the lease
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,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());
}

}

0 comments on commit 6fc8407

Please sign in to comment.