#192 Adding more missing error handling

swisspost · Jul 3, 2024 · 5117707 · 5117707
1 parent e13f77b
commit 5117707
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 8 deletions.
diff --git a/src/main/java/org/swisspush/redisques/action/GetQueueItemAction.java b/src/main/java/org/swisspush/redisques/action/GetQueueItemAction.java
@@ -32,8 +32,8 @@ public void execute(Message<JsonObject> event) {
         String key = queuesPrefix + event.body().getJsonObject(PAYLOAD).getString(QUEUENAME);
         int index = event.body().getJsonObject(PAYLOAD).getInteger(INDEX);
         var p = redisProvider.redis();
-        p.onSuccess(redisAPI -> redisAPI.lindex(key, String.valueOf(index), new GetQueueItemHandler(event)));
-        p.onFailure(ex -> replyErrorMessageHandler(event).handle(ex));
+        p.onSuccess(redisAPI -> redisAPI.lindex(key, String.valueOf(index), new GetQueueItemHandler(event, exceptionFactory)));
+        p.onFailure(ex -> handleFail(event,"Operation GetQueueItemAction failed", ex));
     }
 
 }
diff --git a/src/main/java/org/swisspush/redisques/handler/GetQueueItemHandler.java b/src/main/java/org/swisspush/redisques/handler/GetQueueItemHandler.java
@@ -6,6 +6,7 @@
 import io.vertx.core.json.JsonObject;
 import io.vertx.redis.client.Response;
 import org.slf4j.Logger;
+import org.swisspush.redisques.exception.RedisQuesExceptionFactory;
 
 import static org.slf4j.LoggerFactory.getLogger;
 import static org.swisspush.redisques.util.RedisquesAPI.ERROR;
@@ -22,17 +23,21 @@ public class GetQueueItemHandler implements Handler<AsyncResult<Response>> {
 
     private static final Logger log = getLogger(GetQueueItemHandler.class);
     private final Message<JsonObject> event;
+    private final RedisQuesExceptionFactory exceptionFactory;
 
-    public GetQueueItemHandler(Message<JsonObject> event) {
+    public GetQueueItemHandler(Message<JsonObject> event, RedisQuesExceptionFactory exceptionFactory) {
         this.event = event;
+        this.exceptionFactory = exceptionFactory;
     }
 
     @Override
     public void handle(AsyncResult<Response> reply) {
-        if (reply.succeeded() && reply.result() != null) {
+        if(reply.failed()) {
+            log.warn("Concealed error", exceptionFactory.newException(reply.cause()));
+            event.fail(0, reply.cause().getMessage());
+        } else if (reply.result() != null) {
             event.reply(new JsonObject().put(STATUS, OK).put(VALUE, reply.result().toString()));
         } else {
-            if( reply.failed() ) log.warn("Concealed error", new Exception(reply.cause()));
             event.reply(new JsonObject().put(STATUS, ERROR));
         }
     }

diff --git a/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java b/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java
@@ -759,8 +759,10 @@ private void getSingleQueueItem(RoutingContext ctx) {
                     final int index = Integer.parseInt(lastPart(requestPath));
                     eventBus.request(redisquesAddress, buildGetQueueItemOperation(queue, index), (Handler<AsyncResult<Message<JsonObject>>>) reply -> {
                         if (reply.failed()) {
-                            log.warn("Received failed message for getSingleQueueItemOperation. Lets run into NullPointerException now", reply.cause());
-                            // IMO we should respond with 'HTTP 5xx'. But we don't, to keep backward compatibility.
+                            String error = "Received failed message for getSingleQueueItemOperation";
+                            log.warn(error, exceptionFactory.newException(reply.cause()));
+                            respondWith(StatusCode.INTERNAL_SERVER_ERROR, error, ctx.request());
+                            return;
                         }
                         final JsonObject replyBody = reply.result().body();
                         final HttpServerResponse response = ctx.response();

diff --git a/src/test/java/org/swisspush/redisques/action/GetQueueItemActionTest.java b/src/test/java/org/swisspush/redisques/action/GetQueueItemActionTest.java
@@ -5,6 +5,7 @@
 import io.vertx.core.json.JsonObject;
 import io.vertx.ext.unit.TestContext;
 import io.vertx.ext.unit.junit.VertxUnitRunner;
+import io.vertx.redis.client.impl.types.SimpleStringType;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -41,7 +42,56 @@ public void testGetQueueItemWhenRedisIsNotReady(TestContext context){
 
         action.execute(message);
 
-        verify(message, times(1)).reply(eq(new JsonObject(Buffer.buffer("{\"status\":\"error\"}"))));
+        verify(message, times(1)).fail(eq(0), eq("not ready"));
         verifyNoInteractions(redisAPI);
     }
+
+    @Test
+    public void testGetQueueItem(TestContext context){
+        when(message.body()).thenReturn(buildGetQueueItemOperation("q1", 0));
+
+        doAnswer(invocation -> {
+            var handler = createResponseHandler(invocation,2);
+            handler.handle(Future.succeededFuture(SimpleStringType.create(new JsonObject().put("foo", "bar").encode())));
+            return null;
+        }).when(redisAPI).lindex(anyString(), anyString(), any());
+
+        action.execute(message);
+
+        verify(redisAPI, times(1)).lindex(anyString(), anyString(), any());
+        verify(message, times(1)).reply(eq(new JsonObject(
+                Buffer.buffer("{\"status\":\"ok\",\"value\":\"{\\\"foo\\\":\\\"bar\\\"}\"}"))));
+    }
+
+    @Test
+    public void testGetQueueItemNotExistingIndex(TestContext context){
+        when(message.body()).thenReturn(buildGetQueueItemOperation("q1", 0));
+
+        doAnswer(invocation -> {
+            var handler = createResponseHandler(invocation,2);
+            handler.handle(Future.succeededFuture());
+            return null;
+        }).when(redisAPI).lindex(anyString(), anyString(), any());
+
+        action.execute(message);
+
+        verify(redisAPI, times(1)).lindex(anyString(), anyString(), any());
+        verify(message, times(1)).reply(eq(STATUS_ERROR));
+    }
+
+    @Test
+    public void testGetQueueItemLINDEXFail(TestContext context){
+        when(message.body()).thenReturn(buildGetQueueItemOperation("q1", 0));
+
+        doAnswer(invocation -> {
+            var handler = createResponseHandler(invocation,2);
+            handler.handle(Future.failedFuture("booom"));
+            return null;
+        }).when(redisAPI).lindex(anyString(), anyString(), any());
+
+        action.execute(message);
+
+        verify(redisAPI, times(1)).lindex(anyString(), anyString(), any());
+        verify(message, times(1)).fail(eq(0), eq("booom"));
+    }
 }