Do not retry client requests when failing with ContentTooLargeException (#87248)

client/rest/src/main/java/org/elasticsearch/client/RestClient.java

@@ -21,6 +21,7 @@ package org.elasticsearch.client;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.http.ConnectionClosedException;
+import org.apache.http.ContentTooLongException;
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
@@ -298,7 +299,7 @@ public class RestClient implements Closeable {
             onFailure(context.node);
             Exception cause = extractAndWrapCause(e);
             addSuppressedException(previousException, cause);
-            if (tuple.nodes.hasNext()) {
+            if (isRetryableException(e) && tuple.nodes.hasNext()) {
                 return performRequest(tuple, request, cause);
             }
             if (cause instanceof IOException) {
@@ -414,7 +415,7 @@ public class RestClient implements Closeable {
                     try {
                         RequestLogger.logFailedRequest(logger, request.httpRequest, context.node, failure);
                         onFailure(context.node);
-                        if (tuple.nodes.hasNext()) {
+                        if (isRetryableException(failure) && tuple.nodes.hasNext()) {
                             listener.trackFailure(failure);
                             performRequestAsync(tuple, request, listener);
                         } else {
@@ -563,6 +564,19 @@ public class RestClient implements Closeable {
         return statusCode < 300;
     }
 
+    /**
+     * Should an exception cause retrying the request?
+     */
+    private static boolean isRetryableException(Throwable e) {
+        if (e instanceof ExecutionException) {
+            e = e.getCause();
+        }
+        if (e instanceof ContentTooLongException) {
+            return false;
+        }
+        return true;
+    }
+
     private static boolean isRetryStatus(int statusCode) {
         switch (statusCode) {
             case 502:

client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsIntegTests.java

@@ -31,9 +31,11 @@ import org.junit.BeforeClass;
 import org.junit.Ignore;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.net.ConnectException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -41,6 +43,7 @@ import java.util.concurrent.CancellationException;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.elasticsearch.client.RestClientTestUtil.getAllStatusCodes;
 import static org.elasticsearch.client.RestClientTestUtil.randomErrorNoRetryStatusCode;
@@ -86,10 +89,17 @@ public class RestClientMultipleHostsIntegTests extends RestClientTestCase {
     }
 
     private static RestClient buildRestClient(NodeSelector nodeSelector) {
+        return buildRestClient(nodeSelector, null);
+    }
+
+    private static RestClient buildRestClient(NodeSelector nodeSelector, RestClient.FailureListener failureListener) {
         RestClientBuilder restClientBuilder = RestClient.builder(httpHosts);
         if (pathPrefix.length() > 0) {
             restClientBuilder.setPathPrefix((randomBoolean() ? "/" : "") + pathPrefixWithoutLeadingSlash);
         }
+        if (failureListener != null) {
+            restClientBuilder.setFailureListener(failureListener);
+        }
         restClientBuilder.setNodeSelector(nodeSelector);
         return restClientBuilder.build();
     }
@@ -101,6 +111,7 @@ public class RestClientMultipleHostsIntegTests extends RestClientTestCase {
         for (int statusCode : getAllStatusCodes()) {
             httpServer.createContext(pathPrefix + "/" + statusCode, new ResponseHandler(statusCode));
         }
+        httpServer.createContext(pathPrefix + "/20bytes", new ResponseHandlerWithContent());
         httpServer.createContext(pathPrefix + "/wait", waitForCancelHandler);
         return httpServer;
     }
@@ -153,6 +164,18 @@ public class RestClientMultipleHostsIntegTests extends RestClientTestCase {
         }
     }
 
+    private static class ResponseHandlerWithContent implements HttpHandler {
+        @Override
+        public void handle(HttpExchange httpExchange) throws IOException {
+            byte[] body = "01234567890123456789".getBytes(StandardCharsets.UTF_8);
+            httpExchange.sendResponseHeaders(200, body.length);
+            try (OutputStream out = httpExchange.getResponseBody()) {
+                out.write(body);
+            }
+            httpExchange.close();
+        }
+    }
+
     @AfterClass
     public static void stopHttpServers() throws IOException {
         restClient.close();
@@ -303,6 +326,34 @@ public class RestClientMultipleHostsIntegTests extends RestClientTestCase {
         }
     }
 
+    public void testNonRetryableException() throws Exception {
+        RequestOptions.Builder options = RequestOptions.DEFAULT.toBuilder();
+        options.setHttpAsyncResponseConsumerFactory(
+            // Limit to very short responses to trigger a ContentTooLongException
+            () -> new HeapBufferedAsyncResponseConsumer(10)
+        );
+
+        AtomicInteger failureCount = new AtomicInteger();
+        RestClient client = buildRestClient(NodeSelector.ANY, new RestClient.FailureListener() {
+            @Override
+            public void onFailure(Node node) {
+                failureCount.incrementAndGet();
+            }
+        });
+
+        failureCount.set(0);
+        Request request = new Request("POST", "/20bytes");
+        request.setOptions(options);
+        try {
+            RestClientSingleHostTests.performRequestSyncOrAsync(client, request);
+            fail("Request should not succeed");
+        } catch (IOException e) {
+            assertEquals(stoppedFirstHost ? 2 : 1, failureCount.intValue());
+        }
+
+        client.close();
+    }
+
     private static class TestResponse {
         private final String method;
         private final int statusCode;

docs/changelog/87248.yaml

@@ -0,0 +1,6 @@
+pr: 87248
+summary: Do not retry client requests when failing with `ContentTooLargeException`
+area: Java Low Level REST Client
+type: bug
+issues:
+ - 86041