Handle all of java.net exceptions (#66)

* fix(api-call): handle all java.net exception cases on request fail

* feat(api-all): add optional client to class ctr for dep injection

* chore: add mockito

* test(api-call): add tests for java.net exception handling

- Refactor existing test suite for `ApiCall` class, accessing the
`nodeIndex` static variable, to avoid flaky tests
- Add new test cases ensuring proper handling of all `java.net`
exceptions

* chore: downgrade mockito ver for java 8 compatibility

build.gradle

@@ -107,6 +107,8 @@ dependencies {
     implementation group: 'javax.annotation', name: 'javax.annotation-api', version: '1.3.2'
 
     testImplementation "org.junit.jupiter:junit-jupiter-api:${junitJupiterVersion}"
+    testImplementation "org.mockito:mockito-core:${mokitoVerion}"
+    testImplementation "org.mockito:mockito-junit-jupiter:${mokitoVerion}"
     testImplementation "org.hamcrest:hamcrest-all:${hamcrestVersion}"
     testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:${junitJupiterVersion}"
 

gradle.properties

@@ -7,6 +7,7 @@ hamcrestVersion=1.3
 jacksonCoreVersion=2.14.1
 javaxXmlBindVersion=2.3.1
 junitJupiterVersion=5.9.3
+mokitoVerion=4.11.0
 okhttp3Version=4.10.0
 slf4jVersion=2.0.5
 swaggerCoreV3Version=2.0.0

src/main/java/org/typesense/api/ApiCall.java

@@ -38,6 +38,18 @@ public class ApiCall {
     public static final MediaType JSON = MediaType.parse("application/json; charset=utf-8");
     private final ObjectMapper mapper = new ObjectMapper();
 
+    public ApiCall(Configuration configuration, OkHttpClient client) {
+        this.configuration = configuration;
+        this.nodes = configuration.nodes;
+        this.apiKey = configuration.apiKey;
+        this.retryInterval = configuration.retryInterval;
+
+        mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
+        mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+
+        this.client = client;
+    }
+
     public ApiCall(Configuration configuration) {
         this.configuration = configuration;
         this.nodes = configuration.nodes;
@@ -49,10 +61,10 @@ public ApiCall(Configuration configuration) {
         mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
 
         client = new OkHttpClient()
-                .newBuilder()
-                .connectTimeout(configuration.connectionTimeout.getSeconds(), TimeUnit.SECONDS)
-                .readTimeout(configuration.readTimeout.getSeconds(), TimeUnit.SECONDS)
-                .build();
+            .newBuilder()
+            .connectTimeout(configuration.connectionTimeout.getSeconds(), TimeUnit.SECONDS)
+            .readTimeout(configuration.readTimeout.getSeconds(), TimeUnit.SECONDS)
+            .build();
     }
 
     boolean isDueForHealthCheck(Node node) {
@@ -62,7 +74,7 @@ boolean isDueForHealthCheck(Node node) {
     // Loops in a round-robin fashion to check for a healthy node and returns it
     Node getNode() {
         if (configuration.nearestNode != null) {
-            if (configuration.nearestNode.isHealthy || isDueForHealthCheck((configuration.nearestNode)) ) {
+            if (isDueForHealthCheck((configuration.nearestNode)) || configuration.nearestNode.isHealthy) {
                 return configuration.nearestNode;
             }
         }
@@ -182,8 +194,7 @@ <Q, T> T makeRequest(String endpoint, Q queryParameters, Request.Builder request
             } catch (Exception e) {
                 boolean handleError = (e instanceof ServerError) ||
                                       (e instanceof ServiceUnavailable) ||
-                                      (e instanceof SocketTimeoutException) ||
-                                      (e instanceof java.net.UnknownHostException) ||
+                                      (e.getClass().getPackage().getName().startsWith("java.net")) ||
                                       (e instanceof SSLException);
 
                 if(!handleError) {

src/test/java/org/typesense/api/APICallTest.java

@@ -3,43 +3,73 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.typesense.resources.Node;
 
+import okhttp3.Call;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+
+import java.lang.reflect.Field;
+import java.net.ConnectException;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 class APICallTest {
 
+    @Mock
+    private OkHttpClient client;
+
+    @Mock
+    private Call call;
+
     private ApiCall apiCall;
     private Node nearestNode;
+    private List<Node> nodes;
+
+    @BeforeEach
+    void setUp() {
+        MockitoAnnotations.openMocks(this);
+        nodes = new ArrayList<>();
+        nodes.add(new Node("http", "localhost", "8108"));
+        nodes.add(new Node("http", "localhost", "7108"));
+        nodes.add(new Node("http", "localhost", "6108"));
+    }
 
-    void setUpNoNearestNode() throws Exception {
-        List<Node> nodes = new ArrayList<>();
-        nodes.add(new Node("http","localhost","8108"));
-        nodes.add(new Node("http","localhost","7108"));
-        nodes.add(new Node("http","localhost","6108"));
-        apiCall = new ApiCall(new Configuration(nodes, Duration.ofSeconds(3),"xyz"));
+    private void resetNodeIndex() throws Exception {
+        Field nodeIndexField = ApiCall.class.getDeclaredField("nodeIndex");
+        nodeIndexField.setAccessible(true);
+        nodeIndexField.set(null, 0);
     }
 
-    void setUpNearestNode() throws Exception {
-        List<Node> nodes = new ArrayList<>();
-        nodes.add(new Node("http","localhost","8108"));
-        nodes.add(new Node("http","localhost","7108"));
-        nodes.add(new Node("http","localhost","6108"));
-        nearestNode = new Node("http","localhost","0000");
-        apiCall = new ApiCall(new Configuration(nearestNode, nodes, Duration.ofSeconds(3),"xyz"));
+    void setUpNoNearestNode() {
+        apiCall = new ApiCall(new Configuration(nodes, Duration.ofSeconds(3), "xyz"), client);
+    }
+
+    void setUpNearestNode() {
+        nearestNode = new Node("http", "localhost", "0000");
+        apiCall = new ApiCall(new Configuration(nearestNode, nodes, Duration.ofSeconds(3), "xyz"), client);
     }
 
     @AfterEach
     void tearDown() throws Exception {
-
+        nodes = null;
+        apiCall = null;
+        resetNodeIndex();
     }
 
     @Test
-    void testRoundRobin() throws Exception {
+    void testRoundRobin() {
         setUpNoNearestNode();
         assertEquals("7108", apiCall.getNode().port);
         assertEquals("6108", apiCall.getNode().port);
@@ -50,27 +80,96 @@ void testRoundRobin() throws Exception {
         assertEquals("8108", apiCall.getNode().port);
     }
 
+    @Test
+    void testMakeRequestWithConnectException() throws Exception {
+        setUpNoNearestNode();
+        String endpoint = "/collections";
+        Request.Builder requestBuilder = new Request.Builder().get();
+
+        Call mockCall = mock(Call.class);
+        when(client.newCall(any(Request.class))).thenReturn(mockCall);
+        when(mockCall.execute()).thenThrow(new ConnectException());
+
+        // Act
+        assertThrows(ConnectException.class, () -> {
+            apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
+        });
+
+        // Additional assertions
+        nodes.forEach(node -> {
+            assertEquals(false, node.isHealthy);
+        });
+
+        verify(client, times(3)).newCall(any(Request.class));
+        verify(mockCall, times(3)).execute();
+    }
+
+    @Test
+    void testMakeRequestWithSocketTimeoutException() throws Exception {
+        setUpNoNearestNode();
+        String endpoint = "/collections";
+        Request.Builder requestBuilder = new Request.Builder().get();
+
+        Call mockCall = mock(Call.class);
+        when(client.newCall(any(Request.class))).thenReturn(mockCall);
+        when(mockCall.execute()).thenThrow(new java.net.SocketTimeoutException());
+
+        // Act
+        assertThrows(java.net.SocketTimeoutException.class, () -> {
+            apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
+        });
+
+        // Additional assertions
+        nodes.forEach(node -> {
+            assertEquals(false, node.isHealthy);
+        });
+
+        verify(client, times(3)).newCall(any(Request.class));
+        verify(mockCall, times(3)).execute();
+    }
+
+    @Test
+    void testMakeRequestWithUnknownHostException() throws Exception {
+        setUpNoNearestNode();
+        String endpoint = "/collections";
+        Request.Builder requestBuilder = new Request.Builder().get();
+
+        Call mockCall = mock(Call.class);
+        when(client.newCall(any(Request.class))).thenReturn(mockCall);
+        when(mockCall.execute()).thenThrow(new java.net.UnknownHostException());
+
+        // Act
+        assertThrows(java.net.UnknownHostException.class, () -> {
+            apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
+        });
+
+        // Additional assertions
+        nodes.forEach(node -> {
+            assertEquals(false, node.isHealthy);
+        });
+
+        verify(client, times(3)).newCall(any(Request.class));
+        verify(mockCall, times(3)).execute();
+    }
 
     @Test
-    void testUnhealthyNearestNode() throws Exception {
+    void testUnhealthyNearestNode() {
         setUpNearestNode();
         nearestNode.isHealthy = false;
         assertEquals("7108", apiCall.getNode().port);
     }
 
     @Test
-    void testHealthyNearestNode() throws Exception {
+    void testHealthyNearestNode() {
         setUpNearestNode();
         assertEquals("0000", apiCall.getNode().port);
     }
 
     @Test
-    void testUnhealthyNearestNodeDueForHealthCheck() throws Exception {
+    void testUnhealthyNearestNodeDueForHealthCheck() {
         setUpNearestNode();
         nearestNode.isHealthy = false;
         nearestNode.lastAccessTimestamp = nearestNode.lastAccessTimestamp.minusSeconds(63);
         assertEquals("0000", apiCall.getNode().port);
     }
-
-
 }