http-client-branch: review comment - do not retry non-idempotent requests http-client-branch
authorchegar
Wed, 11 Apr 2018 15:00:35 +0100
branchhttp-client-branch
changeset 56417 312811f70c43
parent 56416 db552808d149
child 56418 56c32f8ea406
http-client-branch: review comment - do not retry non-idempotent requests
src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
test/jdk/java/net/httpclient/ServerCloseTest.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java	Wed Apr 11 14:21:11 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java	Wed Apr 11 15:00:35 2018 +0100
@@ -283,6 +283,37 @@
         return cf;
     }
 
+    private static boolean retryPostValue() {
+        String s = Utils.getNetProperty("jdk.httpclient.enableAllMethodRetry");
+        if (s == "" || "true".equals(s))
+            return true;
+        return false;
+    }
+
+    /** True if ALL ( even non-idempotent ) requests can be automatic retried. */
+    private static final boolean RETRY_ALWAYS = retryPostValue();
+
+    /** Returns true is given request has an idempotent method. */
+    private static boolean isIdempotentRequest(HttpRequest request) {
+        String method = request.method();
+        switch (method) {
+            case "GET" :
+            case "HEAD" :
+                return true;
+            default :
+                return false;
+        }
+    }
+
+    /** Returns true if the given request can be automatically retried. */
+    private static boolean canRetryRequest(HttpRequest request) {
+        if (isIdempotentRequest(request))
+            return true;
+        if (RETRY_ALWAYS)
+            return true;
+        return false;
+    }
+
     /**
      * Takes a Throwable and returns a suitable CompletableFuture that is
      * completed exceptionally, or null.
@@ -296,9 +327,17 @@
         if (cancelled && t instanceof IOException) {
             t = new HttpTimeoutException("request timed out");
         } else if (t instanceof ConnectionExpiredException) {
+            Throwable cause = t;
+            if (t.getCause() != null) {
+                cause = t.getCause(); // unwrap the ConnectionExpiredException
+            }
+
+            if (!canRetryRequest(currentreq)) {
+                return failedFuture(cause); // fails with original cause
+            }
+
             // allow the retry mechanism to do its work
-            // ####: method (GET,HEAD, not POST?), no bytes written or read ( differentiate? )
-            if (t.getCause() != null) retryCause = t.getCause();
+            retryCause = cause;
             if (!expiredOnce) {
                 DEBUG_LOGGER.log(Level.DEBUG,
                     "ConnectionExpiredException (async): retrying...",
--- a/test/jdk/java/net/httpclient/ServerCloseTest.java	Wed Apr 11 14:21:11 2018 +0100
+++ b/test/jdk/java/net/httpclient/ServerCloseTest.java	Wed Apr 11 15:00:35 2018 +0100
@@ -194,8 +194,7 @@
     final String ENCODED = "/01%252F03/";
 
     @Test(dataProvider = "servers")
-    public void testServerClose(String uri, boolean sameClient)
-            throws Exception {
+    public void testServerClose(String uri, boolean sameClient) {
         HttpClient client = null;
         out.printf("%n%s testServerClose(%s, %b)%n", now(), uri, sameClient);
         uri = uri + ENCODED;
@@ -209,7 +208,12 @@
                     .POST(bodyPublisher)
                     .build();
             BodyHandler<String> handler = BodyHandlers.ofString();
-            CompletableFuture<HttpResponse<String>> responseCF = client.sendAsync(req, handler);
+            HttpClient c = client;
+            CompletableFuture<HttpResponse<String>> responseCF = c.sendAsync(req, handler);
+            // retry POST if needed   #### Replace with exceptionallyCompose
+            responseCF = responseCF.handle((r,t) ->
+                    t == null ? CompletableFuture.completedFuture(r)
+                            : c.sendAsync(req, handler)).thenCompose(x -> x);
             HttpResponse<String> response = responseCF.join();
             String body = response.body();
             if (!uri.contains(body)) {