diff --git a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java index 9f5f7740ecd..0d14667f83a 100644 --- a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java @@ -122,11 +122,11 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { catch (ExecutionException ex) { Throwable cause = ex.getCause(); - if (cause instanceof CancellationException) { - if (timeoutHandler != null && timeoutHandler.isTimeout()) { - throw new HttpTimeoutException("Request timed out"); + if (cause instanceof CancellationException ce) { + if (timeoutHandler != null) { + timeoutHandler.handleCancellationException(ce); } - throw new IOException("Request was cancelled"); + throw new IOException("Request cancelled", cause); } if (cause instanceof UncheckedIOException uioEx) { throw uioEx.getCause(); @@ -142,10 +142,10 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { } } catch (CancellationException ex) { - if (timeoutHandler != null && timeoutHandler.isTimeout()) { - throw new HttpTimeoutException("Request timed out"); + if (timeoutHandler != null) { + timeoutHandler.handleCancellationException(ex); } - throw new IOException("Request was cancelled"); + throw new IOException("Request cancelled", ex); } } @@ -244,7 +244,8 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { private static final class TimeoutHandler { private final CompletableFuture timeoutFuture; - private final AtomicBoolean isTimeout = new AtomicBoolean(false); + + private final AtomicBoolean timeout = new AtomicBoolean(false); private TimeoutHandler(CompletableFuture> future, Duration timeout) { @@ -252,8 +253,8 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { .completeOnTimeout(null, timeout.toMillis(), TimeUnit.MILLISECONDS); this.timeoutFuture.thenRun(() -> { + this.timeout.set(true); if (future.cancel(true) || future.isCompletedExceptionally() || !future.isDone()) { - isTimeout.set(true); return; } try { @@ -263,7 +264,6 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { // ignore } }); - } @Nullable @@ -282,8 +282,10 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { }; } - public boolean isTimeout() { - return isTimeout.get(); + public void handleCancellationException(CancellationException ex) throws HttpTimeoutException { + if (this.timeout.get()) { + throw new HttpTimeoutException(ex.getMessage()); + } } } diff --git a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTest.java b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTest.java deleted file mode 100644 index 86630e1fd37..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTest.java +++ /dev/null @@ -1,117 +0,0 @@ -package org.springframework.http.client; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -import java.io.IOException; -import java.io.InputStream; -import java.net.URI; -import java.net.http.HttpClient; -import java.net.http.HttpRequest; -import java.net.http.HttpResponse; -import java.net.http.HttpTimeoutException; -import java.time.Duration; -import java.util.concurrent.*; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; - -class JdkClientHttpRequestTest { - - private HttpClient mockHttpClient; - private URI uri = URI.create("http://example.com"); - private HttpMethod method = HttpMethod.GET; - - private ExecutorService executor; - - @BeforeEach - void setup() { - mockHttpClient = mock(HttpClient.class); - executor = Executors.newSingleThreadExecutor(); - } - - @AfterEach - void tearDown() { - executor.shutdownNow(); - } - - @Test - void executeInternal_withTimeout_shouldThrowHttpTimeoutException() throws Exception { - Duration timeout = Duration.ofMillis(10); - - JdkClientHttpRequest request = new JdkClientHttpRequest(mockHttpClient, uri, method, executor, timeout); - - CompletableFuture> future = new CompletableFuture<>(); - - when(mockHttpClient.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) - .thenReturn(future); - - HttpHeaders headers = new HttpHeaders(); - - CountDownLatch startLatch = new CountDownLatch(1); - - // Cancellation thread waits for startLatch, then cancels the future after a delay - Thread canceller = new Thread(() -> { - try { - startLatch.await(); - Thread.sleep(500); - future.cancel(true); - } catch (InterruptedException ignored) { - } - }); - canceller.start(); - - IOException ex = assertThrows(IOException.class, () -> { - startLatch.countDown(); - request.executeInternal(headers, null); - }); - - assertThat(ex) - .isInstanceOf(HttpTimeoutException.class) - .hasMessage("Request timed out"); - - canceller.join(); - } - - @Test - void executeInternal_withTimeout_shouldThrowIOException() throws Exception { - Duration timeout = Duration.ofMillis(500); - - JdkClientHttpRequest request = new JdkClientHttpRequest(mockHttpClient, uri, method, executor, timeout); - - CompletableFuture> future = new CompletableFuture<>(); - - when(mockHttpClient.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))) - .thenReturn(future); - - HttpHeaders headers = new HttpHeaders(); - - CountDownLatch startLatch = new CountDownLatch(1); - - Thread canceller = new Thread(() -> { - try { - startLatch.await(); - Thread.sleep(10); - future.cancel(true); - } catch (InterruptedException ignored) { - } - }); - canceller.start(); - - IOException ex = assertThrows(IOException.class, () -> { - startLatch.countDown(); - request.executeInternal(headers, null); - }); - - assertThat(ex) - .isInstanceOf(IOException.class) - .hasMessage("Request was cancelled"); - - canceller.join(); - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java new file mode 100644 index 00000000000..1cf59ca5938 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java @@ -0,0 +1,87 @@ +/* + * Copyright 2002-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.http.client; + +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link JdkClientHttpRequest}. + */ +class JdkClientHttpRequestTests { + + private final HttpClient client = mock(HttpClient.class); + + private ExecutorService executor; + + + @BeforeEach + void setup() { + executor = Executors.newSingleThreadExecutor(); + } + + @AfterEach + void tearDown() { + executor.shutdownNow(); + } + + + @Test + void futureCancelledAfterTimeout() { + CompletableFuture> future = new CompletableFuture<>(); + when(client.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(future); + + assertThatThrownBy(() -> createRequest(Duration.ofMillis(10)).executeInternal(new HttpHeaders(), null)) + .isExactlyInstanceOf(HttpTimeoutException.class); + } + + @Test + void futureCancelled() { + CompletableFuture> future = new CompletableFuture<>(); + future.cancel(true); + when(client.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class))).thenReturn(future); + + assertThatThrownBy(() -> createRequest(null).executeInternal(new HttpHeaders(), null)) + .isExactlyInstanceOf(IOException.class); + } + + private JdkClientHttpRequest createRequest(Duration timeout) { + return new JdkClientHttpRequest(client, URI.create("http://abc.com"), HttpMethod.GET, executor, timeout); + } + +}