Java - Add RedisException return to connection/command response handl…

java/client/src/main/java/glide/api/BaseClient.java

@@ -23,11 +23,9 @@ public abstract class BaseClient implements AutoCloseable {
   public void close() throws ExecutionException {
     try {
       connectionManager.closeConnection().get();
-    } catch (InterruptedException interruptedException) {
-      // AutoCloseable classes are strongly advised to avoid throwing InterruptedExceptions
-      // TODO: marking resources as closed:
-      // https://github.com/orgs/Bit-Quill/projects/4/views/6?pane=issue&itemId=48063887
-      throw new RuntimeException(interruptedException);
+    } catch (InterruptedException e) {
+      // suppressing the interrupted exception - it is already suppressed in the future
+      throw new RuntimeException(e);
     }
   }
 }

java/client/src/main/java/glide/api/models/exceptions/ClosingException.java

@@ -0,0 +1,8 @@
+package glide.api.models.exceptions;
+
+/** Error returned from Redis client: Redis is closing or unavailable to the client */
+public class ClosingException extends RedisException {
+  public ClosingException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/api/models/exceptions/ConnectionException.java

@@ -0,0 +1,10 @@
+package glide.api.models.exceptions;
+
+/**
+ * Error returned from Redis client: Redis connection is disconnected or unavailable to the client
+ */
+public class ConnectionException extends RedisException {
+  public ConnectionException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/api/models/exceptions/ExecAbortException.java

@@ -0,0 +1,8 @@
+package glide.api.models.exceptions;
+
+/** Error returned from Redis client: due to transaction execution abort */
+public class ExecAbortException extends RedisException {
+  public ExecAbortException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/api/models/exceptions/RedisException.java

@@ -0,0 +1,8 @@
+package glide.api.models.exceptions;
+
+/** Encapsulated an error returned from the Redis or during processing of a Redis request */
+public class RedisException extends RuntimeException {
+  public RedisException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/api/models/exceptions/RequestException.java

@@ -0,0 +1,8 @@
+package glide.api.models.exceptions;
+
+/** Error returned from Redis client: Redis request has failed */
+public class RequestException extends RedisException {
+  public RequestException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/api/models/exceptions/TimeoutException.java

@@ -0,0 +1,8 @@
+package glide.api.models.exceptions;
+
+/** Error returned from Redis client: request has timed out */
+public class TimeoutException extends RedisException {
+  public TimeoutException(String message) {
+    super(message);
+  }
+}

java/client/src/main/java/glide/connectors/handlers/ChannelHandler.java

@@ -5,6 +5,7 @@
 import glide.connectors.resources.ThreadPoolAllocator;
 import io.netty.bootstrap.Bootstrap;
 import io.netty.channel.Channel;
+import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelInitializer;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.unix.DomainSocketAddress;
@@ -93,8 +94,8 @@ public CompletableFuture<Response> connect(ConnectionRequest request) {
   }
 
   /** Closes the UDS connection and frees corresponding resources. */
-  public void close() {
-    channel.close();
+  public ChannelFuture close() {
     callbackDispatcher.shutdownGracefully();
+    return channel.close();
   }
 }

java/client/src/main/java/glide/managers/CommandManager.java

@@ -1,12 +1,19 @@
 package glide.managers;
 
+import glide.api.models.exceptions.ClosingException;
+import glide.api.models.exceptions.ConnectionException;
+import glide.api.models.exceptions.ExecAbortException;
+import glide.api.models.exceptions.RedisException;
+import glide.api.models.exceptions.RequestException;
+import glide.api.models.exceptions.TimeoutException;
 import glide.connectors.handlers.ChannelHandler;
 import glide.ffi.resolvers.RedisValueResolver;
 import glide.models.RequestBuilder;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import lombok.RequiredArgsConstructor;
 import redis_request.RedisRequestOuterClass.RequestType;
+import response.ResponseOuterClass.RequestError;
 import response.ResponseOuterClass.Response;
 
 /**
@@ -61,20 +68,41 @@ private CompletableFuture<String> submitNewRequest(RequestType command, List<Str
    */
   private String extractValueFromGlideRsResponse(Response response) {
     if (response.hasRequestError()) {
-      // TODO we need to support different types of exceptions and distinguish them by type
-      throw new RuntimeException(
-          String.format(
-              "%s: %s",
-              response.getRequestError().getType(), response.getRequestError().getMessage()));
-    } else if (response.hasClosingError()) {
-      // TODO: close the channel on closingError
-      CompletableFuture.runAsync(channel::close);
-      throw new RuntimeException("Connection closed: " + response.getClosingError());
-    } else if (response.hasConstantResponse()) {
+      RequestError error = response.getRequestError();
+      String msg = error.getMessage();
+      switch (error.getType()) {
+        case Unspecified:
+          // Unspecified error on Redis service-side
+          throw new RequestException(msg);
+        case ExecAbort:
+          // Transactional error on Redis service-side
+          throw new ExecAbortException(msg);
+        case Timeout:
+          // Timeout from Glide to Redis service
+          throw new TimeoutException(msg);
+        case Disconnect:
+          // Connection problem between Glide and Redis
+          throw new ConnectionException(msg);
+        default:
+          // Request or command error from Redis
+          throw new RedisException(msg);
+      }
+    }
+    if (response.hasClosingError()) {
+      // A closing error is thrown when Rust-core is not connected to Redis
+      // We want to close shop and throw a ClosingException
+      channel.close();
+      throw new ClosingException(response.getClosingError());
+    }
+    if (response.hasConstantResponse()) {
+      // Return "OK"
       return response.getConstantResponse().toString();
-    } else if (response.hasRespPointer()) {
+    }
+    if (response.hasRespPointer()) {
+      // Return the shared value - which may be a null value
       return RedisValueResolver.valueFromPointer(response.getRespPointer()).toString();
     }
+    // if no response payload is provided, assume null
     return null;
   }
 }

java/client/src/main/java/glide/managers/ConnectionManager.java

@@ -12,9 +12,13 @@
 import glide.api.models.configuration.ReadFrom;
 import glide.api.models.configuration.RedisClientConfiguration;
 import glide.api.models.configuration.RedisClusterClientConfiguration;
+import glide.api.models.exceptions.ClosingException;
 import glide.connectors.handlers.ChannelHandler;
+import io.netty.channel.ChannelFuture;
+import io.netty.util.concurrent.GenericFutureListener;
 import java.util.concurrent.CompletableFuture;
 import lombok.RequiredArgsConstructor;
+import response.ResponseOuterClass.RequestError;
 import response.ResponseOuterClass.Response;
 
 /**
@@ -162,28 +166,51 @@ private ConnectionRequestOuterClass.ReadFrom mapReadFromEnum(ReadFrom readFrom)
   /** Check a response received from Glide. */
   private Void checkGlideRsResponse(Response response) {
     if (response.hasRequestError()) {
-      // TODO support different types of exceptions and distinguish them by type:
-      throw new RuntimeException(
-          String.format(
-              "%s: %s",
-              response.getRequestError().getType(), response.getRequestError().getMessage()));
+      closeConnection();
+      RequestError error = response.getRequestError();
+      throw new ClosingException("Unexpected request error in response: " + error.getMessage());
     }
     if (response.hasClosingError()) {
-      throw new RuntimeException("Connection closed: " + response.getClosingError());
+      // A closing error is thrown when Rust-core is not connected to Redis
+      // We want to close shop and throw a ClosingException
+      closeConnection();
+      throw new ClosingException(response.getClosingError());
     }
     if (response.hasRespPointer()) {
-      // TODO: throw ClosingException and close/cancel all existing responses
-      throw new RuntimeException("Unexpected data in response");
+      closeConnection();
+      throw new ClosingException("Unexpected data in response");
     }
-    if (response.hasConstantResponse()) {
-      // successful connection response has an "OK"
-      return null;
+    if (!response.hasConstantResponse()) {
+      closeConnection();
+      throw new ClosingException("Unexpected empty data in response");
     }
-    throw new RuntimeException("Connection response expects an OK response");
+    // successful connection response has an "OK"
+    return null;
   }
 
-  /** Close the connection and the corresponding channel. */
+  /** Close the connection to the channel. */
+
+  /**
+   * Close the connection to the channel.
+   *
+   * @return a CompletableFuture to indicate the channel is closed
+   */
   public CompletableFuture<Void> closeConnection() {
-    return CompletableFuture.runAsync(channel::close);
+    CompletableFuture<Void> completableFuture = new CompletableFuture<>();
+    channel
+        .close()
+        .syncUninterruptibly()
+        .addListener(
+            (GenericFutureListener<ChannelFuture>)
+                future -> {
+                  if (future.isCancelled()) {
+                    completableFuture.cancel(false);
+                  } else if (future.isDone()) {
+                    completableFuture.complete(null);
+                  }
+                  completableFuture.completeExceptionally(
+                      new RuntimeException("Channel failed to close"));
+                });
+    return completableFuture;
   }
 }

java/client/src/test/java/glide/api/RedisClientCreateTest.java

@@ -10,6 +10,7 @@
 import static org.mockito.Mockito.when;
 
 import glide.api.models.configuration.RedisClientConfiguration;
+import glide.api.models.exceptions.ClosingException;
 import glide.connectors.handlers.ChannelHandler;
 import glide.managers.CommandManager;
 import glide.managers.ConnectionManager;
@@ -73,8 +74,7 @@ public void createClient_withConfig_successfullyReturnsRedisClient() {
   public void createClient_errorOnConnectionThrowsExecutionException() {
     // setup
     CompletableFuture<Void> connectToRedisFuture = new CompletableFuture<>();
-    // TODO: return a RedisException, not a RuntimeException
-    RuntimeException exception = new RuntimeException("disconnected");
+    ClosingException exception = new ClosingException("disconnected");
     connectToRedisFuture.completeExceptionally(exception);
     RedisClientConfiguration config = RedisClientConfiguration.builder().build();