From f89bb81176f040f6f90b68970dd041a5c4fb5ee0 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Thu, 22 Nov 2018 12:53:56 +1300 Subject: [PATCH] NC-1950: Fixing WebSocket error response (#292) * NC-1950: Fixing WebSocket error response * Adding null id in json rpc error response --- .../websocket/WebSocketRequestHandler.java | 12 ++++++++---- .../WebSocketRequestHandlerTest.java | 19 +++++++++++++++---- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandler.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandler.java index c84b9ef431..7ec02c054d 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandler.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandler.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.jsonrpc.websocket; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.JsonRpcMethod; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.methods.WebSocketRpcRequest; import java.util.Map; @@ -45,12 +46,13 @@ public class WebSocketRequestHandler { request = buffer.toJsonObject().mapTo(WebSocketRpcRequest.class); } catch (final IllegalArgumentException | DecodeException e) { LOG.debug("Error mapping json to WebSocketRpcRequest", e); - future.complete(JsonRpcError.INVALID_REQUEST); + future.complete(new JsonRpcErrorResponse(null, JsonRpcError.INVALID_REQUEST)); return; } if (!methods.containsKey(request.getMethod())) { - future.complete(JsonRpcError.METHOD_NOT_FOUND); + future.complete( + new JsonRpcErrorResponse(request.getId(), JsonRpcError.METHOD_NOT_FOUND)); LOG.debug("Can't find method {}", request.getMethod()); return; } @@ -61,14 +63,16 @@ public class WebSocketRequestHandler { future.complete(method.response(request)); } catch (final Exception e) { LOG.error(JsonRpcError.INTERNAL_ERROR.getMessage(), e); - future.complete(JsonRpcError.INTERNAL_ERROR); + future.complete(new JsonRpcErrorResponse(request.getId(), JsonRpcError.INTERNAL_ERROR)); } }, result -> { if (result.succeeded()) { replyToClient(id, Json.encodeToBuffer(result.result())); } else { - replyToClient(id, Json.encodeToBuffer(JsonRpcError.INTERNAL_ERROR)); + replyToClient( + id, + Json.encodeToBuffer(new JsonRpcErrorResponse(null, JsonRpcError.INTERNAL_ERROR))); } }); } diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandlerTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandlerTest.java index 3e83505b97..2e9499f448 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandlerTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/websocket/WebSocketRequestHandlerTest.java @@ -20,6 +20,7 @@ import static org.mockito.Mockito.when; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.JsonRpcMethod; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.methods.WebSocketRpcRequest; @@ -95,6 +96,9 @@ public class WebSocketRequestHandlerTest { public void jsonDecodeFailureShouldRespondInvalidRequest(final TestContext context) { final Async async = context.async(); + final JsonRpcErrorResponse expectedResponse = + new JsonRpcErrorResponse(null, JsonRpcError.INVALID_REQUEST); + final String websocketId = UUID.randomUUID().toString(); vertx @@ -102,7 +106,7 @@ public class WebSocketRequestHandlerTest { .consumer(websocketId) .handler( msg -> { - context.assertEquals(Json.encode(JsonRpcError.INVALID_REQUEST), msg.body()); + context.assertEquals(Json.encode(expectedResponse), msg.body()); verifyZeroInteractions(jsonRpcMethodMock); async.complete(); }) @@ -115,6 +119,9 @@ public class WebSocketRequestHandlerTest { public void objectMapperFailureShouldRespondInvalidRequest(final TestContext context) { final Async async = context.async(); + final JsonRpcErrorResponse expectedResponse = + new JsonRpcErrorResponse(null, JsonRpcError.INVALID_REQUEST); + final String websocketId = UUID.randomUUID().toString(); vertx @@ -122,7 +129,7 @@ public class WebSocketRequestHandlerTest { .consumer(websocketId) .handler( msg -> { - context.assertEquals(Json.encode(JsonRpcError.INVALID_REQUEST), msg.body()); + context.assertEquals(Json.encode(expectedResponse), msg.body()); verifyZeroInteractions(jsonRpcMethodMock); async.complete(); }) @@ -137,6 +144,8 @@ public class WebSocketRequestHandlerTest { final JsonObject requestJson = new JsonObject().put("id", 1).put("method", "eth_nonexistentMethod"); + final JsonRpcErrorResponse expectedResponse = + new JsonRpcErrorResponse(1, JsonRpcError.METHOD_NOT_FOUND); final String websocketId = UUID.randomUUID().toString(); @@ -145,7 +154,7 @@ public class WebSocketRequestHandlerTest { .consumer(websocketId) .handler( msg -> { - context.assertEquals(Json.encode(JsonRpcError.METHOD_NOT_FOUND), msg.body()); + context.assertEquals(Json.encode(expectedResponse), msg.body()); async.complete(); }) .completionHandler(v -> handler.handle(websocketId, Buffer.buffer(requestJson.toString()))); @@ -160,6 +169,8 @@ public class WebSocketRequestHandlerTest { final JsonObject requestJson = new JsonObject().put("id", 1).put("method", "eth_x"); final JsonRpcRequest expectedRequest = requestJson.mapTo(WebSocketRpcRequest.class); when(jsonRpcMethodMock.response(eq(expectedRequest))).thenThrow(new RuntimeException()); + final JsonRpcErrorResponse expectedResponse = + new JsonRpcErrorResponse(1, JsonRpcError.INTERNAL_ERROR); final String websocketId = UUID.randomUUID().toString(); @@ -168,7 +179,7 @@ public class WebSocketRequestHandlerTest { .consumer(websocketId) .handler( msg -> { - context.assertEquals(Json.encode(JsonRpcError.INTERNAL_ERROR), msg.body()); + context.assertEquals(Json.encode(expectedResponse), msg.body()); async.complete(); }) .completionHandler(v -> handler.handle(websocketId, Buffer.buffer(requestJson.toString())));