When an HTTP client request timeout fires, the corresponding stream i…

…s 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.
eclipse-vertx · Aug 21, 2023 · 17f6dce · 17f6dce
1 parent 0fc43be
commit 17f6dce
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 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
@@ -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<HttpClientResponse> 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
@@ -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<Throwable> handler = exceptionHandler();

diff --git 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();
           }