Skip to content

Commit 6b38076

Browse files
committed
fix: use timeoutHandler with a flag isTimeout
1 parent 1717ef7 commit 6b38076

File tree

2 files changed

+132
-4
lines changed

2 files changed

+132
-4
lines changed

spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.concurrent.Executor;
3838
import java.util.concurrent.Flow;
3939
import java.util.concurrent.TimeUnit;
40+
import java.util.concurrent.atomic.AtomicBoolean;
4041

4142
import org.jspecify.annotations.Nullable;
4243

@@ -121,7 +122,10 @@ protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body
121122
Throwable cause = ex.getCause();
122123

123124
if (cause instanceof CancellationException) {
124-
throw new HttpTimeoutException("Request timed out");
125+
if (timeoutHandler != null && timeoutHandler.isTimeout()) {
126+
throw new HttpTimeoutException("Request timed out");
127+
}
128+
throw new IOException("Request was cancelled");
125129
}
126130
if (cause instanceof UncheckedIOException uioEx) {
127131
throw uioEx.getCause();
@@ -138,7 +142,10 @@ else if (cause instanceof IOException ioEx) {
138142
}
139143
}
140144
catch (CancellationException ex) {
141-
throw new HttpTimeoutException("Request timed out");
145+
if (timeoutHandler != null && timeoutHandler.isTimeout()) {
146+
throw new HttpTimeoutException("Request timed out");
147+
}
148+
throw new IOException("Request was cancelled");
142149
}
143150
}
144151

@@ -228,7 +235,7 @@ public ByteBuffer map(byte[] b, int off, int len) {
228235
private static final class TimeoutHandler {
229236

230237
private final CompletableFuture<Void> timeoutFuture;
231-
private boolean isTimeout=false;
238+
private final AtomicBoolean isTimeout = new AtomicBoolean(false);
232239

233240
private TimeoutHandler(CompletableFuture<HttpResponse<InputStream>> future, Duration timeout) {
234241

@@ -237,7 +244,7 @@ private TimeoutHandler(CompletableFuture<HttpResponse<InputStream>> future, Dura
237244

238245
this.timeoutFuture.thenRun(() -> {
239246
if (future.cancel(true) || future.isCompletedExceptionally() || !future.isDone()) {
240-
this.isTimeout = true;
247+
isTimeout.set(true);
241248
return;
242249
}
243250
try {
@@ -264,6 +271,10 @@ public void close() throws IOException {
264271
}
265272
};
266273
}
274+
275+
public boolean isTimeout() {
276+
return isTimeout.get();
277+
}
267278
}
268279

269280
}
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package org.springframework.http.client;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.junit.jupiter.api.Assertions.*;
5+
import static org.mockito.Mockito.*;
6+
7+
import java.io.IOException;
8+
import java.io.InputStream;
9+
import java.net.URI;
10+
import java.net.http.HttpClient;
11+
import java.net.http.HttpRequest;
12+
import java.net.http.HttpResponse;
13+
import java.net.http.HttpTimeoutException;
14+
import java.time.Duration;
15+
import java.util.concurrent.*;
16+
17+
import org.junit.jupiter.api.AfterEach;
18+
import org.junit.jupiter.api.BeforeEach;
19+
import org.junit.jupiter.api.Test;
20+
import org.springframework.http.HttpHeaders;
21+
import org.springframework.http.HttpMethod;
22+
23+
class JdkClientHttpRequestTest {
24+
25+
private HttpClient mockHttpClient;
26+
private URI uri = URI.create("http://example.com");
27+
private HttpMethod method = HttpMethod.GET;
28+
29+
private ExecutorService executor;
30+
31+
@BeforeEach
32+
void setup() {
33+
mockHttpClient = mock(HttpClient.class);
34+
executor = Executors.newSingleThreadExecutor();
35+
}
36+
37+
@AfterEach
38+
void tearDown() {
39+
executor.shutdownNow();
40+
}
41+
42+
@Test
43+
void executeInternal_withTimeout_shouldThrowHttpTimeoutException() throws Exception {
44+
Duration timeout = Duration.ofMillis(10);
45+
46+
JdkClientHttpRequest request = new JdkClientHttpRequest(mockHttpClient, uri, method, executor, timeout);
47+
48+
CompletableFuture<HttpResponse<InputStream>> future = new CompletableFuture<>();
49+
50+
when(mockHttpClient.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class)))
51+
.thenReturn(future);
52+
53+
HttpHeaders headers = new HttpHeaders();
54+
55+
CountDownLatch startLatch = new CountDownLatch(1);
56+
57+
// Cancellation thread waits for startLatch, then cancels the future after a delay
58+
Thread canceller = new Thread(() -> {
59+
try {
60+
startLatch.await();
61+
Thread.sleep(500);
62+
future.cancel(true);
63+
} catch (InterruptedException ignored) {
64+
}
65+
});
66+
canceller.start();
67+
68+
IOException ex = assertThrows(IOException.class, () -> {
69+
startLatch.countDown();
70+
request.executeInternal(headers, null);
71+
});
72+
73+
assertThat(ex)
74+
.isInstanceOf(HttpTimeoutException.class)
75+
.hasMessage("Request timed out");
76+
77+
canceller.join();
78+
}
79+
80+
@Test
81+
void executeInternal_withTimeout_shouldThrowIOException() throws Exception {
82+
Duration timeout = Duration.ofMillis(500);
83+
84+
JdkClientHttpRequest request = new JdkClientHttpRequest(mockHttpClient, uri, method, executor, timeout);
85+
86+
CompletableFuture<HttpResponse<InputStream>> future = new CompletableFuture<>();
87+
88+
when(mockHttpClient.sendAsync(any(HttpRequest.class), any(HttpResponse.BodyHandler.class)))
89+
.thenReturn(future);
90+
91+
HttpHeaders headers = new HttpHeaders();
92+
93+
CountDownLatch startLatch = new CountDownLatch(1);
94+
95+
Thread canceller = new Thread(() -> {
96+
try {
97+
startLatch.await();
98+
Thread.sleep(10);
99+
future.cancel(true);
100+
} catch (InterruptedException ignored) {
101+
}
102+
});
103+
canceller.start();
104+
105+
IOException ex = assertThrows(IOException.class, () -> {
106+
startLatch.countDown();
107+
request.executeInternal(headers, null);
108+
});
109+
110+
assertThat(ex)
111+
.isInstanceOf(IOException.class)
112+
.hasMessage("Request was cancelled");
113+
114+
canceller.join();
115+
}
116+
117+
}

0 commit comments

Comments
 (0)