NullPointerException on startInclusive when using multiple nodes in cluster mode

[stwonary]

Description:

When using the Typesense Java SDK (version 0.8.0) with a cluster setup, I encounter a NullPointerException when inserting documents:

Cannot invoke "java.time.temporal.Temporal.until(java.time.temporal.Temporal, java.time.temporal.TemporalUnit)" because "startInclusive" is null

I believe the problem lies in this condition in Client.java:

if (this.configuration.nearestNode == null || !this.isDueForHealthCheck(this.configuration.nearestNode) && !this.configuration.nearestNode.isHealthy) {

The default value of isHealthy is true, but lastAccessTimestamp is null, as seen here:

https://github.com/typesense/typesense-java/blob/27bacf7937a085ede7306bd57a70d705d6c00c32/src/main/java/org/typesense/resources/Node.java#L23-L42

This results in the Duration.between() ApiCall.java:58 method throwing an exception.

---

Steps to Reproduce:

1. Set up a multi-node cluster with Typesense Cloud:

   • Node 1: xxx-1.a1.typesense.net
   • Node 2: xxx-2.a1.typesense.net
   • Node 3: xxx-3.a1.typesense.net
   • Load-balanced endpoint: xxx.a1.typesense.net

2. Use the Java SDK (version 0.8.0) with the following configuration:

   ArrayList<Node> nodes = new ArrayList<>();
   nodes.add(new Node("https", "xxx-1.a1.typesense.net", "443"));
   nodes.add(new Node("https", "xxx-2.a1.typesense.net", "443"));
   nodes.add(new Node("https", "xxx-3.a1.typesense.net", "443"));
   
   Node nearestNode = new Node("https", "xxx.a1.typesense.net", "443");
   
   Configuration configuration = new Configuration(nearestNode, nodes, Duration.ofSeconds(2), "API_KEY");
   Client client = new Client(configuration);

3. Attempt to insert a document into the companies collection.

---

Expected Behavior:

Document insertion should succeed.

---

Actual Behavior:

A NullPointerException occurs due to the lastAccessTimestamp being null when multiple nodes are used.

---

Metadata:

Typesense Version: 27.0 (Typesense Cloud)
Java SDK Version: 0.8.0

[stwonary]

Just tried with the latest version 0.8.1 and got the same error.

[tharropoulos]

This error would be eliminated by prioritizing the condition of the node being healthy in the check. Here's why:

The early exit strategy, when the nearest node is found to be healthy (as is the case for the first access of a node), would prevent unnecessary checks for if a node is due for a health check. Consequently, if a node's request would resolve in an network error, it would set the health check to false:

https://github.com/typesense/typesense-java/blob/27bacf7937a085ede7306bd57a70d705d6c00c32/src/main/java/org/typesense/api/ApiCall.java#L193

By utilizing the setNodeHealthStatus method, we ensure that the last access timestamp is updated to the current time. This approach prevents the value from being null from that point onwards:

https://github.com/typesense/typesense-java/blob/27bacf7937a085ede7306bd57a70d705d6c00c32/src/main/java/org/typesense/api/ApiCall.java#L86-L89

In pull request #62, we address not only the order of checks performed but also add an initial value to a Node when constructed. This addition helps avoid potential null reference errors in the future. Given that there's a single export of the client using a Configuration whose lifetime matches that of the application, setting all nodes' initial access timestamps to their creation time doesn't pose a problem. This approach's effectiveness is demonstrated in the tests provided in the PR.

[kishorenc]

Please check on 0.9.0 that has just been published.

[stwonary]

I’ve just tested version 0.9.0, and everything is working perfectly. Thank you!