Review feedback from @japai

dfuch · Jan 14, 2025 · ff98d87 · ff98d87
1 parent 0be4389
commit ff98d87
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 8 deletions.
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java
@@ -331,7 +331,7 @@ public <U> CompletableFuture<U> readBody(HttpResponse.BodySubscriber<U> p,
                 );
                 if (cf.isCompletedExceptionally()) {
                     // if an error occurs during subscription
-                    connection.close(cf.exceptionNow());
+                    connection.close(Utils.exceptionNow(cf));
                     return;
                 }
                 // increment the reference count on the HttpClientImpl

diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java
@@ -57,6 +57,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ConcurrentHashMap;
@@ -1133,4 +1134,31 @@ public static String encode(String s) {
         }
         return sb.toString();
     }
+
+    /**
+     * {@return the exception the given {@code cf} was completed with,
+     * or a {@link CancellationException} if the given {@code cf} was
+     * cancelled}
+     *
+     * @param cf a {@code CompletableFuture} exceptionally completed
+     * @throws IllegalArgumentException if the given cf was not
+     *    {@linkplain CompletableFuture#isCompletedExceptionally()
+     *    completed exceptionally}
+     */
+    public static Throwable exceptionNow(CompletableFuture<?> cf) {
+        if (cf.isCompletedExceptionally()) {
+            if (cf.isCancelled()) {
+                try {
+                    cf.join();
+                } catch (CancellationException x) {
+                    return x;
+                } catch (CompletionException x) {
+                    return x.getCause();
+                }
+            } else {
+                return cf.exceptionNow();
+            }
+        }
+        throw new IllegalArgumentException("cf is not completed exceptionally");
+    }
 }
diff --git a/test/jdk/java/net/httpclient/http2/ExpectContinueResetTest.java b/test/jdk/java/net/httpclient/http2/ExpectContinueResetTest.java
@@ -93,17 +93,20 @@ public void test(URI uri) {
         err.printf("\nTesting with Version: %s, URI: %s\n", HTTP_2, uri.toASCIIString());
         Iterable<byte[]> iterable = EndlessDataChunks::new;
         HttpRequest.BodyPublisher testPub = HttpRequest.BodyPublishers.ofByteArrays(iterable);
-        Exception exception = null;
-        Throwable testThrowable = null;
+        Exception expectedException = null;
         try {
             performRequest(testPub, uri);
+            throw new AssertionError("Expected exception not raised for " + uri);
         } catch (Exception e) {
-            exception = e;
-            testThrowable = e.getCause();
+            expectedException = e;
         }
-        assertNotNull(exception, "Request should have completed exceptionally but exception is null");
-        assertNotNull(testThrowable, "Request should have completed exceptionally but testThrowable is null");
-        assertEquals(testThrowable.getClass(), IOException.class, "Test should have closed with an IOException");
+        Throwable testThrowable = expectedException.getCause();
+        if (testThrowable == null) {
+            throw new AssertionError("Unexpected null cause for " + expectedException,
+                    expectedException);
+        }
+        assertEquals(testThrowable.getClass(), IOException.class,
+                "Test should have closed with an IOException");
         testThrowable.printStackTrace();
     }