diff --git a/src/main/java/org/swisspush/redisques/action/GetLockAction.java b/src/main/java/org/swisspush/redisques/action/GetLockAction.java index bcc0858..012830b 100644 --- a/src/main/java/org/swisspush/redisques/action/GetLockAction.java +++ b/src/main/java/org/swisspush/redisques/action/GetLockAction.java @@ -39,9 +39,9 @@ public void execute(Message event) { } var p = redisProvider.redis(); p.onSuccess(redisAPI -> { - redisAPI.hget(locksKey, body.getJsonObject(PAYLOAD).getString(QUEUENAME), new GetLockHandler(event)); + redisAPI.hget(locksKey, body.getJsonObject(PAYLOAD).getString(QUEUENAME), new GetLockHandler(event, exceptionFactory)); }); - p.onFailure(ex -> replyErrorMessageHandler(event).handle(ex)); + p.onFailure(ex -> handleFail(event,"Operation GetLockAction failed", ex)); } } diff --git a/src/main/java/org/swisspush/redisques/handler/GetLockHandler.java b/src/main/java/org/swisspush/redisques/handler/GetLockHandler.java index d40b673..7d4a9b9 100644 --- a/src/main/java/org/swisspush/redisques/handler/GetLockHandler.java +++ b/src/main/java/org/swisspush/redisques/handler/GetLockHandler.java @@ -6,9 +6,9 @@ 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; import static org.swisspush.redisques.util.RedisquesAPI.NO_SUCH_LOCK; import static org.swisspush.redisques.util.RedisquesAPI.OK; import static org.swisspush.redisques.util.RedisquesAPI.STATUS; @@ -23,9 +23,11 @@ public class GetLockHandler implements Handler> { private static final Logger log = getLogger(GetLockHandler.class); private final Message event; + private final RedisQuesExceptionFactory exceptionFactory; - public GetLockHandler(Message event) { + public GetLockHandler(Message event, RedisQuesExceptionFactory exceptionFactory) { this.event = event; + this.exceptionFactory = exceptionFactory; } @Override @@ -37,8 +39,8 @@ public void handle(AsyncResult reply) { event.reply(new JsonObject().put(STATUS, NO_SUCH_LOCK)); } } else { - log.warn("Concealed error", new Exception(reply.cause())); - event.reply(new JsonObject().put(STATUS, ERROR)); + log.warn("Concealed error", exceptionFactory.newException(reply.cause())); + event.fail(0, reply.cause().getMessage()); } } diff --git a/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java b/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java index 8d8cef5..f423873 100644 --- a/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java +++ b/src/main/java/org/swisspush/redisques/handler/RedisquesHttpRequestHandler.java @@ -358,10 +358,10 @@ private void getSingleLock(RoutingContext ctx) { queue -> eventBus.request(redisquesAddress, buildGetLockOperation(queue), (Handler>>) reply -> { final HttpServerResponse response = ctx.response(); if (reply.failed()) { - log.warn("Received failed message for getSingleLockOperation. Lets run into NullPointerException now", reply.cause()); - // IMO we should respond with 'HTTP 5xx' here. But we don't, to keep backward compatibility. - } - if (OK.equals(reply.result().body().getString(STATUS))) { + String error = "Received failed message for getSingleLockOperation"; + log.warn(error, exceptionFactory.newException(reply.cause())); + respondWith(StatusCode.INTERNAL_SERVER_ERROR, error, ctx.request()); + } else if (OK.equals(reply.result().body().getString(STATUS))) { response.putHeader(CONTENT_TYPE, APPLICATION_JSON); response.end(reply.result().body().getString(VALUE)); } else { diff --git a/src/test/java/org/swisspush/redisques/action/GetLockActionTest.java b/src/test/java/org/swisspush/redisques/action/GetLockActionTest.java index abc6935..7d383af 100644 --- a/src/test/java/org/swisspush/redisques/action/GetLockActionTest.java +++ b/src/test/java/org/swisspush/redisques/action/GetLockActionTest.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 testGetLockWhenRedisIsNotReady(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 testGetLock(TestContext context){ + when(message.body()).thenReturn(buildGetLockOperation("q1")); + + doAnswer(invocation -> { + var handler = createResponseHandler(invocation,2); + handler.handle(Future.succeededFuture(SimpleStringType.create(new JsonObject().put("requestedBy", "UNKNOWN").put("timestamp", 1719931433522L).encode()))); + return null; + }).when(redisAPI).hget(anyString(), anyString(), any()); + + action.execute(message); + + verify(redisAPI, times(1)).hget(anyString(), anyString(), any()); + verify(message, times(1)).reply(eq(new JsonObject(Buffer.buffer("{\"status\":\"ok\"," + + "\"value\":\"{\\\"requestedBy\\\":\\\"UNKNOWN\\\",\\\"timestamp\\\":1719931433522}\"}")))); + } + + @Test + public void testGetLockNoSuchLock(TestContext context){ + when(message.body()).thenReturn(buildGetLockOperation("q1")); + + doAnswer(invocation -> { + var handler = createResponseHandler(invocation,2); + handler.handle(Future.succeededFuture()); + return null; + }).when(redisAPI).hget(anyString(), anyString(), any()); + + action.execute(message); + + verify(redisAPI, times(1)).hget(anyString(), anyString(), any()); + verify(message, times(1)).reply(eq(new JsonObject(Buffer.buffer("{\"status\":\"No such lock\"}")))); + } + + @Test + public void testGetLockHGETFail(TestContext context){ + when(message.body()).thenReturn(buildGetLockOperation("q1")); + + doAnswer(invocation -> { + var handler = createResponseHandler(invocation,2); + handler.handle(Future.failedFuture("booom")); + return null; + }).when(redisAPI).hget(anyString(), anyString(), any()); + + action.execute(message); + + verify(redisAPI, times(1)).hget(anyString(), anyString(), any()); + verify(message, times(1)).fail(eq(0), eq("booom")); + } }