Skip to content
/ besu Public
forked from hyperledger/besu

Commit

Permalink
Improve PluginRpcEndpointException log (hyperledger#6789)
Browse files Browse the repository at this point in the history
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: amsmota <antonio.mota@citi.com>
  • Loading branch information
fab-10 authored and amsmota committed Apr 16, 2024
1 parent b7ba727 commit 54791fd
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ public JsonRpcResponse response(final JsonRpcRequestContext request) {
final Object result = function.apply(() -> request.getRequest().getParams());
return new JsonRpcSuccessResponse(request.getRequest().getId(), result);
} catch (final PluginRpcEndpointException ex) {
final JsonRpcError error = new JsonRpcError(ex.getRpcMethodError(), ex.getMessage());
LOG.error("Error calling plugin JSON-RPC endpoint", ex);
final JsonRpcError error = new JsonRpcError(ex.getRpcMethodError(), ex.getData());
LOG.debug("Error calling plugin JSON-RPC endpoint", ex);
return new JsonRpcErrorResponse(request.getRequest().getId(), error);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,16 @@ protected void assertValidJsonRpcResult(final JsonObject json, final Object id)
}

protected void assertValidJsonRpcError(
final JsonObject json, final Object id, final int errorCode, final String errorMessage)
throws Exception {
final JsonObject json, final Object id, final int errorCode, final String errorMessage) {
assertValidJsonRpcError(json, id, errorCode, errorMessage, null);
}

protected void assertValidJsonRpcError(
final JsonObject json,
final Object id,
final int errorCode,
final String errorMessage,
final String data) {
// Check all expected fieldnames are set
final Set<String> fieldNames = json.fieldNames();
assertThat(fieldNames.size()).isEqualTo(3);
Expand All @@ -53,13 +61,19 @@ protected void assertValidJsonRpcError(
// Check error format
final JsonObject error = json.getJsonObject("error");
final Set<String> errorFieldNames = error.fieldNames();
assertThat(errorFieldNames.size()).isEqualTo(2);
assertThat(errorFieldNames.size()).isEqualTo(data == null ? 2 : 3);
assertThat(errorFieldNames.contains("code")).isTrue();
assertThat(errorFieldNames.contains("message")).isTrue();
if (data != null) {
assertThat(errorFieldNames.contains("data")).isTrue();
}

// Check error field values
assertThat(error.getInteger("code")).isEqualTo(errorCode);
assertThat(error.getString("message")).isEqualTo(errorMessage);
if (data != null) {
assertThat(error.getString("data")).isEqualTo(data);
}
}

protected void assertIdMatches(final JsonObject json, final Object expectedId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
import org.hyperledger.besu.plugin.services.rpc.PluginRpcRequest;
import org.hyperledger.besu.plugin.services.rpc.RpcMethodError;

import java.util.Locale;
import java.util.Optional;

import io.vertx.core.json.JsonObject;
import okhttp3.RequestBody;
import okhttp3.Response;
Expand Down Expand Up @@ -127,6 +130,25 @@ public void methodErrorShouldReturnErrorResponse() throws Exception {
}
}

@Test
public void methodErrorWithDataShouldReturnErrorResponseWithDecodedData() throws Exception {
final var wrongParamContent =
"""
{"jsonrpc":"2.0","id":1,"method":"plugin_echo","params":["data"]}""";
try (var unused =
addRpcMethod(
"plugin_echo",
new PluginJsonRpcMethod("plugin_echo", PluginJsonRpcMethodTest::echoPluginRpcMethod))) {
final RequestBody body = RequestBody.create(wrongParamContent, JSON);

try (final Response resp = client.newCall(buildPostRequest(body)).execute()) {
assertThat(resp.code()).isEqualTo(200);
final JsonObject json = new JsonObject(resp.body().string());
testHelper.assertValidJsonRpcError(json, 1, -2, "Error with data: ABC", "abc");
}
}
}

@Test
public void unhandledExceptionShouldReturnInternalErrorResponse() throws Exception {
final var nullParam =
Expand Down Expand Up @@ -168,6 +190,29 @@ public String getMessage() {
}
});
}

if (input.toString().equals("data")) {
throw new PluginRpcEndpointException(
new RpcMethodError() {
@Override
public int getCode() {
return -2;
}

@Override
public String getMessage() {
return "Error with data";
}

@Override
public Optional<String> decodeData(final String data) {
// just turn everything uppercase
return Optional.of(data.toUpperCase(Locale.US));
}
},
"abc");
}

return input;
}
}
2 changes: 1 addition & 1 deletion plugin-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Calculated : ${currentHash}
tasks.register('checkAPIChanges', FileStateChecker) {
description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
files = sourceSets.main.allJava.files
knownHash = '0xiYCyr3M4oSrvqYXVkLgVDzlBg2T3fmrADub5tY5a0='
knownHash = '/FHIztl2tLW5Gzc0qnfEeuVQa6ljVfUce7YE6JLDdZU='
}
check.dependsOn('checkAPIChanges')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,44 +20,45 @@
public class PluginRpcEndpointException extends RuntimeException {
/** The error */
private final RpcMethodError rpcMethodError;
/** The data associated with the exception */
private final String data;

/**
* Constructs a new PluginRpcEndpointException exception with the specified error.
*
* @param rpcMethodError the error.
*/
public PluginRpcEndpointException(final RpcMethodError rpcMethodError) {
super();
this.rpcMethodError = rpcMethodError;
this(rpcMethodError, null);
}

/**
* Constructs a new PluginRpcEndpointException exception with the specified error and message.
*
* @param rpcMethodError the error.
* @param message the detail message (which is saved for later retrieval by the {@link
* #getMessage()} method).
* @param data the data associated with the exception that could be parsed to extract more
* information to return in the error response.
*/
public PluginRpcEndpointException(final RpcMethodError rpcMethodError, final String message) {
super(message);
this.rpcMethodError = rpcMethodError;
public PluginRpcEndpointException(final RpcMethodError rpcMethodError, final String data) {
this(rpcMethodError, data, null);
}

/**
* Constructs a new PluginRpcEndpointException exception with the specified error, message and
* cause.
*
* @param rpcMethodError the error.
* @param message the detail message (which is saved for later retrieval by the {@link
* #getMessage()} method).
* @param data the data associated with the exception that could be parsed to extract more
* information to return in the error response.
* @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method).
* (A {@code null} value is permitted, and indicates that the cause is nonexistent or
* unknown.)
*/
public PluginRpcEndpointException(
final RpcMethodError rpcMethodError, final String message, final Throwable cause) {
super(message, cause);
final RpcMethodError rpcMethodError, final String data, final Throwable cause) {
super(rpcMethodError.getMessage(), cause);
this.rpcMethodError = rpcMethodError;
this.data = data;
}

/**
Expand All @@ -68,4 +69,13 @@ public PluginRpcEndpointException(
public RpcMethodError getRpcMethodError() {
return rpcMethodError;
}

/**
* Get the data associated with the exception
*
* @return data as string, could be null.
*/
public String getData() {
return data;
}
}

0 comments on commit 54791fd

Please sign in to comment.