Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve PluginRpcEndpointException #6789

Merged
merged 1 commit into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
Loading