Skip to content

Commit 3cc78d4

Browse files
committed
The HttpClientRequest implementation does have a mapping of HTTP closed exception when a timeout occurs so the client is signaled with a stream exception for better usability. Actually we can generify this for the generic reset exception case instead of just being for idle timeouts.
1 parent f17dcbc commit 3cc78d4

File tree

4 files changed

+36
-28
lines changed

4 files changed

+36
-28
lines changed

src/main/java/io/vertx/core/http/impl/HttpClientRequestBase.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public abstract class HttpClientRequestBase implements HttpClientRequest {
4545
private long currentTimeoutTimerId = -1;
4646
private long currentTimeoutMs;
4747
private long lastDataReceived;
48-
private Throwable timeoutFired;
48+
private Throwable reset;
4949

5050
HttpClientRequestBase(HttpClientStream stream, PromiseInternal<HttpClientResponse> responsePromise, HttpMethod method, SocketAddress server, String host, int port, String uri) {
5151
this.stream = stream;
@@ -167,8 +167,8 @@ public synchronized HttpClientRequest setTimeout(long timeout) {
167167
}
168168

169169
protected Throwable mapException(Throwable t) {
170-
if (t instanceof HttpClosedException && timeoutFired != null) {
171-
t = timeoutFired;
170+
if (t instanceof HttpClosedException && reset != null) {
171+
t = reset;
172172
}
173173
return t;
174174
}
@@ -196,7 +196,9 @@ void handlePush(HttpClientPush push) {
196196
}
197197

198198
void handleResponse(HttpClientResponse resp) {
199-
handleResponse(responsePromise, resp, cancelTimeout());
199+
if (reset == null) {
200+
handleResponse(responsePromise, resp, cancelTimeout());
201+
}
200202
}
201203

202204
abstract void handleResponse(Promise<HttpClientResponse> promise, HttpClientResponse resp, long timeoutMs);
@@ -228,7 +230,6 @@ private void handleTimeout(long timeoutMs) {
228230
}
229231
}
230232
cause = timeoutEx(timeoutMs, method, server, uri);
231-
timeoutFired = cause;
232233
}
233234
reset(cause);
234235
}
@@ -253,7 +254,16 @@ public boolean reset(long code, Throwable cause) {
253254
return reset(new StreamResetException(code, cause));
254255
}
255256

256-
abstract boolean reset(Throwable cause);
257+
private boolean reset(Throwable cause) {
258+
synchronized (this) {
259+
if (reset != null) {
260+
return false;
261+
}
262+
reset = cause;
263+
}
264+
stream.reset(cause);
265+
return true;
266+
}
257267

258268
@Override
259269
public HttpClientRequest response(Handler<AsyncResult<HttpClientResponse>> handler) {

src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public class HttpClientRequestImpl extends HttpClientRequestBase implements Http
5454
private Handler<Throwable> exceptionHandler;
5555
private Function<HttpClientResponse, Future<HttpClientRequest>> redirectHandler;
5656
private boolean ended;
57-
private Throwable reset;
5857
private boolean followRedirects;
5958
private int maxRedirects;
6059
private int numberOfRedirections;
@@ -297,18 +296,6 @@ public String traceOperation() {
297296
return traceOperation;
298297
}
299298

300-
@Override
301-
boolean reset(Throwable cause) {
302-
synchronized (this) {
303-
if (reset != null) {
304-
return false;
305-
}
306-
reset = cause;
307-
}
308-
stream.reset(cause);
309-
return true;
310-
}
311-
312299
private void tryComplete() {
313300
endPromise.tryComplete();
314301
}
@@ -379,9 +366,6 @@ private void handleEarlyHints(MultiMap headers) {
379366
}
380367

381368
void handleResponse(Promise<HttpClientResponse> promise, HttpClientResponse resp, long timeoutMs) {
382-
if (reset != null) {
383-
return;
384-
}
385369
int statusCode = resp.statusCode();
386370
if (followRedirects && numberOfRedirections < maxRedirects && statusCode >= 300 && statusCode < 400) {
387371
Function<HttpClientResponse, Future<HttpClientRequest>> handler = redirectHandler;

src/main/java/io/vertx/core/http/impl/HttpClientRequestPushPromise.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,6 @@ public HttpConnection connection() {
5959
return stream.connection();
6060
}
6161

62-
@Override
63-
boolean reset(Throwable cause) {
64-
stream.reset(cause);
65-
return true;
66-
}
67-
6862
@Override
6963
public boolean isChunked() {
7064
return false;

src/test/java/io/vertx/core/http/Http2Test.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,4 +1072,24 @@ public void testRstFloodProtection() throws Exception {
10721072
}
10731073
await();
10741074
}
1075+
1076+
@Test
1077+
public void testStreamResetErrorMapping() throws Exception {
1078+
server.requestHandler(req -> {
1079+
});
1080+
startServer(testAddress);
1081+
client.request(requestOptions).onComplete(onSuccess(req -> {
1082+
req.exceptionHandler(err -> {
1083+
assertTrue(err instanceof StreamResetException);
1084+
StreamResetException sre = (StreamResetException) err;
1085+
assertEquals(10, sre.getCode());
1086+
testComplete();
1087+
});
1088+
// Force stream allocation
1089+
req.sendHead().onComplete(onSuccess(v -> {
1090+
req.reset(10);
1091+
}));
1092+
}));
1093+
await();
1094+
}
10751095
}

0 commit comments

Comments
 (0)