Skip to content

Commit 53c71b5

Browse files
tharropoulostharropoulos
authored
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
1 parent 129935f commit 53c71b5

File tree

4 files changed

+140
-27
lines changed

4 files changed

+140
-27
lines changed

‎build.gradle‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ dependencies {
107107
implementation group: 'javax.annotation', name: 'javax.annotation-api', version: '1.3.2'
108108

109109
testImplementation "org.junit.jupiter:junit-jupiter-api:${junitJupiterVersion}"
110+
testImplementation "org.mockito:mockito-core:${mokitoVerion}"
111+
testImplementation "org.mockito:mockito-junit-jupiter:${mokitoVerion}"
110112
testImplementation "org.hamcrest:hamcrest-all:${hamcrestVersion}"
111113
testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:${junitJupiterVersion}"
112114

‎gradle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ hamcrestVersion=1.3
77
jacksonCoreVersion=2.14.1
88
javaxXmlBindVersion=2.3.1
99
junitJupiterVersion=5.9.3
10+
mokitoVerion=4.11.0
1011
okhttp3Version=4.10.0
1112
slf4jVersion=2.0.5
1213
swaggerCoreV3Version=2.0.0

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ public class ApiCall {
3838
public static final MediaType JSON = MediaType.parse("application/json; charset=utf-8");
3939
private final ObjectMapper mapper = new ObjectMapper();
4040

41+
public ApiCall(Configuration configuration, OkHttpClient client) {
42+
this.configuration = configuration;
43+
this.nodes = configuration.nodes;
44+
this.apiKey = configuration.apiKey;
45+
this.retryInterval = configuration.retryInterval;
46+
47+
mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
48+
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
49+
50+
this.client = client;
51+
}
52+
4153
public ApiCall(Configuration configuration) {
4254
this.configuration = configuration;
4355
this.nodes = configuration.nodes;
@@ -49,10 +61,10 @@ public ApiCall(Configuration configuration) {
4961
mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
5062

5163
client = new OkHttpClient()
52-
.newBuilder()
53-
.connectTimeout(configuration.connectionTimeout.getSeconds(), TimeUnit.SECONDS)
54-
.readTimeout(configuration.readTimeout.getSeconds(), TimeUnit.SECONDS)
55-
.build();
64+
.newBuilder()
65+
.connectTimeout(configuration.connectionTimeout.getSeconds(), TimeUnit.SECONDS)
66+
.readTimeout(configuration.readTimeout.getSeconds(), TimeUnit.SECONDS)
67+
.build();
5668
}
5769

5870
boolean isDueForHealthCheck(Node node) {
@@ -62,7 +74,7 @@ boolean isDueForHealthCheck(Node node) {
6274
// Loops in a round-robin fashion to check for a healthy node and returns it
6375
Node getNode() {
6476
if (configuration.nearestNode != null) {
65-
if (configuration.nearestNode.isHealthy || isDueForHealthCheck((configuration.nearestNode)) ) {
77+
if (isDueForHealthCheck((configuration.nearestNode)) || configuration.nearestNode.isHealthy) {
6678
return configuration.nearestNode;
6779
}
6880
}
@@ -182,8 +194,7 @@ <Q, T> T makeRequest(String endpoint, Q queryParameters, Request.Builder request
182194
} catch (Exception e) {
183195
boolean handleError = (e instanceof ServerError) ||
184196
(e instanceof ServiceUnavailable) ||
185-
(e instanceof SocketTimeoutException) ||
186-
(e instanceof java.net.UnknownHostException) ||
197+
(e.getClass().getPackage().getName().startsWith("java.net")) ||
187198
(e instanceof SSLException);
188199

189200
if(!handleError) {

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

Lines changed: 119 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,43 +3,73 @@
33
import org.junit.jupiter.api.AfterEach;
44
import org.junit.jupiter.api.BeforeEach;
55
import org.junit.jupiter.api.Test;
6+
import org.mockito.Mock;
7+
import org.mockito.MockitoAnnotations;
68
import org.typesense.resources.Node;
79

10+
import okhttp3.Call;
11+
import okhttp3.OkHttpClient;
12+
import okhttp3.Request;
13+
14+
import java.lang.reflect.Field;
15+
import java.net.ConnectException;
816
import java.time.Duration;
917
import java.util.ArrayList;
1018
import java.util.List;
1119

1220
import static org.junit.jupiter.api.Assertions.assertEquals;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
22+
import static org.mockito.ArgumentMatchers.any;
23+
import static org.mockito.Mockito.mock;
24+
import static org.mockito.Mockito.times;
25+
import static org.mockito.Mockito.verify;
26+
import static org.mockito.Mockito.when;
1327

1428
class APICallTest {
1529

30+
@Mock
31+
private OkHttpClient client;
32+
33+
@Mock
34+
private Call call;
35+
1636
private ApiCall apiCall;
1737
private Node nearestNode;
38+
private List<Node> nodes;
39+
40+
@BeforeEach
41+
void setUp() {
42+
MockitoAnnotations.openMocks(this);
43+
nodes = new ArrayList<>();
44+
nodes.add(new Node("http", "localhost", "8108"));
45+
nodes.add(new Node("http", "localhost", "7108"));
46+
nodes.add(new Node("http", "localhost", "6108"));
47+
}
1848

19-
void setUpNoNearestNode() throws Exception {
20-
List<Node> nodes = new ArrayList<>();
21-
nodes.add(new Node("http","localhost","8108"));
22-
nodes.add(new Node("http","localhost","7108"));
23-
nodes.add(new Node("http","localhost","6108"));
24-
apiCall = new ApiCall(new Configuration(nodes, Duration.ofSeconds(3),"xyz"));
49+
private void resetNodeIndex() throws Exception {
50+
Field nodeIndexField = ApiCall.class.getDeclaredField("nodeIndex");
51+
nodeIndexField.setAccessible(true);
52+
nodeIndexField.set(null, 0);
2553
}
2654

27-
void setUpNearestNode() throws Exception {
28-
List<Node> nodes = new ArrayList<>();
29-
nodes.add(new Node("http","localhost","8108"));
30-
nodes.add(new Node("http","localhost","7108"));
31-
nodes.add(new Node("http","localhost","6108"));
32-
nearestNode = new Node("http","localhost","0000");
33-
apiCall = new ApiCall(new Configuration(nearestNode, nodes, Duration.ofSeconds(3),"xyz"));
55+
void setUpNoNearestNode() {
56+
apiCall = new ApiCall(new Configuration(nodes, Duration.ofSeconds(3), "xyz"), client);
57+
}
58+
59+
void setUpNearestNode() {
60+
nearestNode = new Node("http", "localhost", "0000");
61+
apiCall = new ApiCall(new Configuration(nearestNode, nodes, Duration.ofSeconds(3), "xyz"), client);
3462
}
3563

3664
@AfterEach
3765
void tearDown() throws Exception {
38-
66+
nodes = null;
67+
apiCall = null;
68+
resetNodeIndex();
3969
}
4070

4171
@Test
42-
void testRoundRobin() throws Exception {
72+
void testRoundRobin() {
4373
setUpNoNearestNode();
4474
assertEquals("7108", apiCall.getNode().port);
4575
assertEquals("6108", apiCall.getNode().port);
@@ -50,27 +80,96 @@ void testRoundRobin() throws Exception {
5080
assertEquals("8108", apiCall.getNode().port);
5181
}
5282

83+
@Test
84+
void testMakeRequestWithConnectException() throws Exception {
85+
setUpNoNearestNode();
86+
String endpoint = "/collections";
87+
Request.Builder requestBuilder = new Request.Builder().get();
88+
89+
Call mockCall = mock(Call.class);
90+
when(client.newCall(any(Request.class))).thenReturn(mockCall);
91+
when(mockCall.execute()).thenThrow(new ConnectException());
92+
93+
// Act
94+
assertThrows(ConnectException.class, () -> {
95+
apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
96+
});
97+
98+
// Additional assertions
99+
nodes.forEach(node -> {
100+
assertEquals(false, node.isHealthy);
101+
});
102+
103+
verify(client, times(3)).newCall(any(Request.class));
104+
verify(mockCall, times(3)).execute();
105+
}
106+
107+
@Test
108+
void testMakeRequestWithSocketTimeoutException() throws Exception {
109+
setUpNoNearestNode();
110+
String endpoint = "/collections";
111+
Request.Builder requestBuilder = new Request.Builder().get();
112+
113+
Call mockCall = mock(Call.class);
114+
when(client.newCall(any(Request.class))).thenReturn(mockCall);
115+
when(mockCall.execute()).thenThrow(new java.net.SocketTimeoutException());
116+
117+
// Act
118+
assertThrows(java.net.SocketTimeoutException.class, () -> {
119+
apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
120+
});
121+
122+
// Additional assertions
123+
nodes.forEach(node -> {
124+
assertEquals(false, node.isHealthy);
125+
});
126+
127+
verify(client, times(3)).newCall(any(Request.class));
128+
verify(mockCall, times(3)).execute();
129+
}
130+
131+
@Test
132+
void testMakeRequestWithUnknownHostException() throws Exception {
133+
setUpNoNearestNode();
134+
String endpoint = "/collections";
135+
Request.Builder requestBuilder = new Request.Builder().get();
136+
137+
Call mockCall = mock(Call.class);
138+
when(client.newCall(any(Request.class))).thenReturn(mockCall);
139+
when(mockCall.execute()).thenThrow(new java.net.UnknownHostException());
140+
141+
// Act
142+
assertThrows(java.net.UnknownHostException.class, () -> {
143+
apiCall.makeRequest(endpoint, null, requestBuilder, String.class);
144+
});
145+
146+
// Additional assertions
147+
nodes.forEach(node -> {
148+
assertEquals(false, node.isHealthy);
149+
});
150+
151+
verify(client, times(3)).newCall(any(Request.class));
152+
verify(mockCall, times(3)).execute();
153+
}
53154

54155
@Test
55-
void testUnhealthyNearestNode() throws Exception {
156+
void testUnhealthyNearestNode() {
56157
setUpNearestNode();
57158
nearestNode.isHealthy = false;
58159
assertEquals("7108", apiCall.getNode().port);
59160
}
60161

61162
@Test
62-
void testHealthyNearestNode() throws Exception {
163+
void testHealthyNearestNode() {
63164
setUpNearestNode();
64165
assertEquals("0000", apiCall.getNode().port);
65166
}
66167

67168
@Test
68-
void testUnhealthyNearestNodeDueForHealthCheck() throws Exception {
169+
void testUnhealthyNearestNodeDueForHealthCheck() {
69170
setUpNearestNode();
70171
nearestNode.isHealthy = false;
71172
nearestNode.lastAccessTimestamp = nearestNode.lastAccessTimestamp.minusSeconds(63);
72173
assertEquals("0000", apiCall.getNode().port);
73174
}
74-
75-
76175
}

0 commit comments

Comments
 (0)