[DE-411] http retries (#468)

* CodeQL fixes

* added http retries for safe methods

Author: rashtao

.github/workflows/codeql.yml

@@ -28,6 +28,11 @@ jobs:
   analyze:
     name: Analyze
     runs-on: ubuntu-latest
+
+    defaults:
+      run:
+        working-directory: driver
+
     permissions:
       actions: read
       contents: read

driver/src/main/java/com/arangodb/internal/http/HttpCommunication.java

@@ -22,6 +22,7 @@
 
 import com.arangodb.ArangoDBException;
 import com.arangodb.Request;
+import com.arangodb.RequestType;
 import com.arangodb.Response;
 import com.arangodb.internal.net.*;
 import com.arangodb.internal.serde.InternalSerde;
@@ -84,8 +85,6 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
                     return response;
                 } catch (final SocketTimeoutException e) {
                     // SocketTimeoutException exceptions are wrapped and rethrown.
-                    // Differently from other IOException exceptions they must not be retried,
-                    // since the requests could not be idempotent.
                     TimeoutException te = new TimeoutException(e.getMessage());
                     te.initCause(e);
                     throw new ArangoDBException(te, reqId);
@@ -96,7 +95,7 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
                     }
                     final Host failedHost = host;
                     host = hostHandler.get(hostHandle, accessType);
-                    if (host != null) {
+                    if (host != null && isSafe(request)) {
                         LOGGER.warn("Could not connect to {} while executing request [id={}]",
                                 failedHost.getDescription(), reqId, e);
                         LOGGER.debug("Try connecting to {}", host.getDescription());
@@ -106,9 +105,9 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
                     }
                 }
             }
-        } catch (final ArangoDBException e) {
-            if (e instanceof ArangoDBRedirectException && attemptCount < 3) {
-                final String location = ((ArangoDBRedirectException) e).getLocation();
+        } catch (final ArangoDBRedirectException e) {
+            if (attemptCount < 3) {
+                final String location = e.getLocation();
                 final HostDescription redirectHost = HostUtils.createFromLocation(location);
                 hostHandler.failIfNotMatch(redirectHost, e);
                 return execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1);
@@ -118,6 +117,11 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
         }
     }
 
+    private boolean isSafe(final Request request) {
+        RequestType type = request.getRequestType();
+        return type == RequestType.GET || type == RequestType.HEAD || type == RequestType.OPTIONS;
+    }
+
     public static class Builder {
         private HostHandler hostHandler;
         private InternalSerde serde;

driver/src/main/java/com/arangodb/internal/http/HttpConnection.java

@@ -61,6 +61,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
 
 
@@ -227,10 +228,12 @@ public Response execute(final Request request) throws IOException {
             throw ArangoDBException.wrap(e);
         } catch (ExecutionException e) {
             Throwable cause = e.getCause();
-            if (cause instanceof IOException) {
+            if (cause instanceof TimeoutException) {
+                throw ArangoDBException.wrap(cause);
+            } else if (cause instanceof IOException) {
                 throw (IOException) cause;
             } else {
-                throw ArangoDBException.wrap(e.getCause());
+                throw new IOException(cause);
             }
         }
         checkError(resp);

resilience-tests/src/test/java/resilience/SingleServerTest.java

@@ -37,8 +37,8 @@ static void afterAll() throws IOException {
     }
 
     @BeforeEach
-    void beforeEach() throws IOException {
-        endpoint.getProxy().enable();
+    void beforeEach() {
+        enableEndpoint();
     }
 
     protected static Endpoint getEndpoint() {
@@ -57,4 +57,22 @@ protected static ArangoDBAsync.Builder dbBuilderAsync() {
                 .password(PASSWORD);
     }
 
+    protected void enableEndpoint(){
+        try {
+            getEndpoint().getProxy().enable();
+            Thread.sleep(100);
+        } catch (IOException | InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    protected void disableEndpoint(){
+        try {
+            getEndpoint().getProxy().disable();
+            Thread.sleep(100);
+        } catch (IOException | InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
 }

resilience-tests/src/test/java/resilience/connection/ConnectionTest.java

@@ -8,7 +8,6 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
-import java.io.IOException;
 import java.net.ConnectException;
 import java.net.UnknownHostException;
 import java.util.stream.Stream;
@@ -59,21 +58,18 @@ void nameResolutionFailTest(Protocol protocol) {
 
     @ParameterizedTest
     @MethodSource("arangoProvider")
-    void connectionFailTest(ArangoDB arangoDB) throws IOException, InterruptedException {
-        getEndpoint().getProxy().disable();
-        Thread.sleep(100);
+    void connectionFailTest(ArangoDB arangoDB) {
+        disableEndpoint();
 
         Throwable thrown = catchThrowable(arangoDB::getVersion);
         assertThat(thrown).isInstanceOf(ArangoDBException.class);
         assertThat(thrown.getMessage()).contains("Cannot contact any host");
         assertThat(thrown.getCause()).isNotNull();
         assertThat(thrown.getCause()).isInstanceOf(ArangoDBMultipleException.class);
-        ((ArangoDBMultipleException) thrown.getCause()).getExceptions().forEach(e -> {
-            assertThat(e).isInstanceOf(ConnectException.class);
-        });
+        ((ArangoDBMultipleException) thrown.getCause()).getExceptions().forEach(e ->
+                assertThat(e).isInstanceOf(ConnectException.class));
         arangoDB.shutdown();
-        getEndpoint().getProxy().enable();
-        Thread.sleep(100);
+        enableEndpoint();
     }
 
 }