Implement HTTP retry logic and set connection timeout in RESTClient

jsign-crypto/pom.xml

@@ -53,6 +53,13 @@
       <version>1.3.1</version>
       <scope>test</scope>
     </dependency>
+
+    <dependency>
+      <groupId>com.github.tomakehurst</groupId>
+      <artifactId>wiremock-standalone</artifactId>
+      <version>2.27.2</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

jsign-crypto/src/main/java/net/jsign/jca/RESTClient.java

@@ -18,6 +18,7 @@
 
 import java.io.IOException;
 import java.net.HttpURLConnection;
+import java.net.SocketTimeoutException;
 import java.net.URL;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
@@ -47,6 +48,18 @@ class RESTClient {
     /** Callback building an error message from the JSON formatted error response */
     private Function<Map<String, ?>, String> errorHandler;
 
+    /** Connect timeout (in milliseconds) */
+    private int connectTimeout = 30000;
+
+    /** Read timeout (in milliseconds) */
+    private int readTimeout = 30000;
+
+    /** Number of retries */
+    private int retries = 3;
+
+    /** Pause between retries (in milliseconds) */
+    private int retryPause = 5000;
+
     public RESTClient(String endpoint) {
         this.endpoint = endpoint;
     }
@@ -66,6 +79,42 @@ public RESTClient errorHandler(Function<Map<String, ?>, String> errorHandler) {
         return this;
     }
 
+    /**
+     * Sets the connect timeout.
+     *
+     * @param connectTimeout the timeout in milliseconds
+     */
+    public void setConnectTimeout(int connectTimeout) {
+        this.connectTimeout = connectTimeout;
+    }
+
+    /**
+     * Sets the read timeout.
+     *
+     * @param readTimeout the timeout in milliseconds
+     */
+    public void setReadTimeout(int readTimeout) {
+        this.readTimeout = readTimeout;
+    }
+
+    /**
+     * Sets the number of retries.
+     *
+     * @param retries the number of retries
+     */
+    public void setRetries(int retries) {
+        this.retries = retries;
+    }
+
+    /**
+     * Sets the pause between retries.
+     *
+     * @param retryPause the pause in milliseconds
+     */
+    public void setRetryPause(int retryPause) {
+        this.retryPause = retryPause;
+    }
+
     public Map<String, ?> get(String resource) throws IOException {
         return query("GET", resource, null, null);
     }
@@ -124,9 +173,31 @@ public RESTClient errorHandler(Function<Map<String, ?>, String> errorHandler) {
     }
 
     private Map<String, ?> query(String method, String resource, String body, Map<String, String> headers) throws IOException {
+        int attempts = 0;
+        while (true) {
+            try {
+                return queryOnce(method, resource, body, headers);
+            } catch (SocketTimeoutException e) {
+                attempts++;
+                if (attempts >= retries) {
+                    throw e;
+                }
+                log.warning("Connection timeout, retrying in " + (retryPause / 1000) + " seconds (attempt " + attempts + "/" + retries + ")");
+                try {
+                    Thread.sleep(retryPause);
+                } catch (InterruptedException ie) {
+                    Thread.currentThread().interrupt();
+                }
+            }
+        }
+    }
+
+    private Map<String, ?> queryOnce(String method, String resource, String body, Map<String, String> headers) throws IOException {
         URL url = new URL(resource.startsWith("http") ? resource : endpoint + resource);
         log.finest(method + " " + url);
         HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+        conn.setConnectTimeout(connectTimeout);
+        conn.setReadTimeout(readTimeout);
         conn.setRequestMethod(method);
         String userAgent = System.getProperty("http.agent");
         conn.setRequestProperty("User-Agent", "Jsign (https://ebourg.github.io/jsign/)" + (userAgent != null ? " " + userAgent : ""));

jsign-crypto/src/test/java/net/jsign/jca/RESTClientTest.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2024 Emmanuel Bourg
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.jsign.jca;
+
+import java.io.IOException;
+import java.net.SocketTimeoutException;
+import java.util.Map;
+
+import com.github.tomakehurst.wiremock.WireMockServer;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.*;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.junit.Assert.*;
+
+public class RESTClientTest {
+
+    private WireMockServer wireMockServer;
+
+    @Before
+    public void setUp() {
+        wireMockServer = new WireMockServer(wireMockConfig().dynamicPort());
+        wireMockServer.start();
+    }
+
+    @After
+    public void tearDown() {
+        wireMockServer.stop();
+    }
+
+    @Test
+    public void testRetryOnTimeout() {
+        wireMockServer.stubFor(get(urlEqualTo("/test"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withFixedDelay(1000)));
+
+        RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port());
+        client.setReadTimeout(100);
+        client.setRetries(3);
+        client.setRetryPause(10);
+
+        assertThrows(SocketTimeoutException.class, () -> client.get("/test"));
+        wireMockServer.verify(3, getRequestedFor(urlEqualTo("/test")));
+    }
+
+    @Test
+    public void testRetryEventuallySucceeds() throws Exception {
+        wireMockServer.stubFor(get(urlEqualTo("/test")).inScenario("Retry Scenario")
+                .whenScenarioStateIs("Started")
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withFixedDelay(500))
+                .willSetStateTo("Succeeded"));
+
+        wireMockServer.stubFor(get(urlEqualTo("/test")).inScenario("Retry Scenario")
+                .whenScenarioStateIs("Succeeded")
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withHeader("Content-Type", "application/json")
+                        .withBody("{\"status\":\"ok\"}")));
+
+        RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port());
+        client.setReadTimeout(200);
+        client.setRetries(4); // allow enough retries
+        client.setRetryPause(400);
+
+        Map<String, ?> response = client.get("/test");
+        assertEquals("ok", response.get("status"));
+        wireMockServer.verify(2, getRequestedFor(urlEqualTo("/test")));
+    }
+
+    @Test
+    public void testNoRetryOnOtherException() {
+        wireMockServer.stubFor(get(urlEqualTo("/test"))
+                .willReturn(aResponse()
+                        .withStatus(404)));
+
+        RESTClient client = new RESTClient("http://localhost:" + wireMockServer.port());
+        client.setRetries(3);
+        client.setRetryPause(10);
+
+        assertThrows(IOException.class, () -> client.get("/test"));
+        wireMockServer.verify(1, getRequestedFor(urlEqualTo("/test")));
+    }
+}