From 17f6dce25473665b796d983a59015b1dda14f289 Mon Sep 17 00:00:00 2001 From: Julien Viet Date: Fri, 11 Aug 2023 14:02:19 +0200 Subject: [PATCH] When an HTTP client request timeout fires, the corresponding stream is reset with a cancel code, the application cannot be aware of the cancellation because the netty stream reset will simply notify the stream to be closed. We can keep track of the timeout in the HttpClientRequest and replace the HTTP closed exception with the timeout exception when this happens to better inform the application. --- .../core/http/impl/HttpClientRequestBase.java | 18 +++++++++++++----- .../core/http/impl/HttpClientRequestImpl.java | 1 + src/test/java/io/vertx/core/http/HttpTest.java | 8 ++------ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/vertx/core/http/impl/HttpClientRequestBase.java b/src/main/java/io/vertx/core/http/impl/HttpClientRequestBase.java index 1b288a0bd61..ce601b0dcdc 100644 --- a/src/main/java/io/vertx/core/http/impl/HttpClientRequestBase.java +++ b/src/main/java/io/vertx/core/http/impl/HttpClientRequestBase.java @@ -16,10 +16,7 @@ import io.vertx.core.Future; import io.vertx.core.Handler; import io.vertx.core.Promise; -import io.vertx.core.http.HttpClientRequest; -import io.vertx.core.http.HttpClientResponse; -import io.vertx.core.http.HttpMethod; -import io.vertx.core.http.StreamResetException; +import io.vertx.core.http.*; import io.vertx.core.impl.ContextInternal; import io.vertx.core.impl.future.PromiseInternal; import io.vertx.core.net.HostAndPort; @@ -48,6 +45,7 @@ public abstract class HttpClientRequestBase implements HttpClientRequest { private long currentTimeoutTimerId = -1; private long currentTimeoutMs; private long lastDataReceived; + private Throwable timeoutFired; HttpClientRequestBase(HttpClientImpl client, HttpClientStream stream, PromiseInternal responsePromise, boolean ssl, HttpMethod method, SocketAddress server, String host, int port, String uri) { this.client = client; @@ -169,6 +167,13 @@ public synchronized HttpClientRequest setTimeout(long timeoutMs) { return this; } + protected Throwable mapException(Throwable t) { + if (t instanceof HttpClosedException && timeoutFired != null) { + t = timeoutFired; + } + return t; + } + void handleException(Throwable t) { fail(t); } @@ -209,6 +214,7 @@ private synchronized long cancelTimeout() { } private void handleTimeout(long timeoutMs) { + NoStackTraceTimeoutException cause; synchronized (this) { currentTimeoutTimerId = -1; currentTimeoutMs = 0; @@ -222,8 +228,10 @@ private void handleTimeout(long timeoutMs) { return; } } + cause = timeoutEx(timeoutMs, method, server, uri); + timeoutFired = cause; } - reset(timeoutEx(timeoutMs, method, server, uri)); + reset(cause); } static NoStackTraceTimeoutException timeoutEx(long timeoutMs, HttpMethod method, SocketAddress server, String uri) { diff --git a/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java b/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java index 965f6e18d92..c97dce40774 100644 --- a/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java +++ b/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java @@ -77,6 +77,7 @@ public class HttpClientRequestImpl extends HttpClientRequestBase implements Http @Override void handleException(Throwable t) { + t = mapException(t); super.handleException(t); if (endPromise.tryFail(t)) { Handler handler = exceptionHandler(); diff --git a/src/test/java/io/vertx/core/http/HttpTest.java b/src/test/java/io/vertx/core/http/HttpTest.java index a1e6a8e9e6d..65ac81a4f3b 100644 --- a/src/test/java/io/vertx/core/http/HttpTest.java +++ b/src/test/java/io/vertx/core/http/HttpTest.java @@ -3200,9 +3200,7 @@ public void testResponseDataTimeout() { AtomicInteger count = new AtomicInteger(); resp.exceptionHandler(t -> { if (count.getAndIncrement() == 0) { - assertTrue( - t instanceof TimeoutException || /* HTTP/1 */ - t instanceof VertxException /* HTTP/2: connection closed */); + assertTrue(t instanceof TimeoutException); assertEquals(expected, received); complete(); } @@ -3222,9 +3220,7 @@ public void testResponseDataTimeout() { AtomicInteger count = new AtomicInteger(); req.exceptionHandler(t -> { if (count.getAndIncrement() == 0) { - assertTrue( - t instanceof TimeoutException || /* HTTP/1 */ - t instanceof VertxException /* HTTP/2: connection closed */); + assertTrue(t instanceof TimeoutException); assertEquals(expected, received); complete(); }