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

Fix JSON-RPC Errors in ATs #1428

Merged
merged 3 commits into from Oct 8, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -10,7 +10,7 @@ Prior versions of Besu would set the HTTP Status 400 Bad Request for JSON-RPC re

In Besu version 20.10, properly formatted requests that have valid parameters (count and content) will return a HTTP Status 200 OK, with an error field if an error occurred. For example, requesting an account that does not exist in the chain, or a block by hash that Besu does not have, will now return HTTP 200 OK responses. Unparsable requests, improperly formatted requests, or requests with invalid parameters will continue to return HTTP 400 Bad Request.

This was done to bring us more in line with the behavior of other Ethereum Clients. Some community projects, such as Web3J, will be providing compatible releases in the near future.
Users of Web3J should note that many calls will now return a result with the error field containing the message whereas before a call would throw an exception with the error message as the exception message.

## 20.10.0-RC1

Expand Down
Expand Up @@ -21,8 +21,6 @@
import org.hyperledger.besu.tests.acceptance.dsl.node.Node;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.eth.EthAccountsTransaction;

import org.web3j.protocol.exceptions.ClientConnectionException;

public class ExpectEthAccountsException implements Condition {

private final String expectedMessage;
Expand All @@ -37,7 +35,7 @@ public ExpectEthAccountsException(
@Override
public void verify(final Node node) {
final Throwable thrown = catchThrowable(() -> node.execute(transaction));
assertThat(thrown).isInstanceOf(ClientConnectionException.class);
assertThat(thrown).isInstanceOf(RuntimeException.class);
assertThat(thrown.getMessage()).contains(expectedMessage);
}
}
Expand Up @@ -21,8 +21,6 @@
import org.hyperledger.besu.tests.acceptance.dsl.node.Node;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.eth.EthGetWorkTransaction;

import org.web3j.protocol.exceptions.ClientConnectionException;

public class ExpectEthGetWorkException implements Condition {

private final EthGetWorkTransaction transaction;
Expand All @@ -37,7 +35,7 @@ public ExpectEthGetWorkException(
@Override
public void verify(final Node node) {
final Throwable thrown = catchThrowable(() -> node.execute(transaction));
assertThat(thrown).isInstanceOf(ClientConnectionException.class);
assertThat(thrown).isInstanceOf(RuntimeException.class);
assertThat(thrown.getMessage()).contains(expectedMessage);
}
}
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.tests.acceptance.dsl.condition.eth;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchThrowable;

import org.hyperledger.besu.tests.acceptance.dsl.condition.Condition;
import org.hyperledger.besu.tests.acceptance.dsl.node.Node;
Expand All @@ -33,8 +34,8 @@ public ExpectEthSendRawTransactionException(

@Override
public void verify(final Node node) {
final String result = node.execute(transaction);
assertThat(result).isNotNull();
assertThat(result).contains(expectedMessage);
final Throwable thrown = catchThrowable(() -> node.execute(transaction));
assertThat(thrown).isInstanceOf(RuntimeException.class);
assertThat(thrown.getMessage()).contains(expectedMessage);
}
}
Expand Up @@ -41,8 +41,7 @@ public void verify(final Node node) {
failBecauseExceptionWasNotThrown(ClientConnectionException.class);
} catch (final Exception e) {
Assertions.assertThat(e)
.isInstanceOf(ClientConnectionException.class)
.hasMessageContaining("400")
.isInstanceOf(RuntimeException.class)
.hasMessageContaining(error.getMessage());
}
}
Expand Down
Expand Up @@ -19,8 +19,6 @@
import org.hyperledger.besu.tests.acceptance.dsl.privacy.PrivacyNode;
import org.hyperledger.besu.tests.acceptance.dsl.privacy.transaction.PrivacyTransactions;

import org.web3j.protocol.exceptions.ClientConnectionException;

public class ExpectInternalErrorPrivateTransactionReceipt implements PrivateCondition {
private final PrivacyTransactions transactions;
private final String transactionHash;
Expand All @@ -35,7 +33,7 @@ public ExpectInternalErrorPrivateTransactionReceipt(
public void verify(final PrivacyNode node) {
try {
node.execute(transactions.getPrivateTransactionReceipt(transactionHash));
} catch (final ClientConnectionException e) {
} catch (final RuntimeException e) {
assertThat(e.getMessage()).contains("Internal error");
}
}
Expand Down
Expand Up @@ -24,6 +24,7 @@
import java.util.stream.Collectors;

import org.web3j.protocol.besu.Besu;
import org.web3j.protocol.besu.response.privacy.PrivCreatePrivacyGroup;
import org.web3j.utils.Base64String;

public class CreatePrivacyGroupTransaction implements Transaction<String> {
Expand All @@ -45,11 +46,14 @@ public CreatePrivacyGroupTransaction(
public String execute(final NodeRequests node) {
final Besu besu = node.privacy().getBesuClient();
try {
return besu.privCreatePrivacyGroup(addresses, name, description)
.send()
.getPrivacyGroupId()
.toString();
} catch (IOException e) {
final PrivCreatePrivacyGroup result =
besu.privCreatePrivacyGroup(addresses, name, description).send();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
} else {
return result.getPrivacyGroupId().toString();
}
} catch (final IOException e) {
throw new RuntimeException(e);
}
}
Expand Down
Expand Up @@ -14,9 +14,12 @@
*/
package org.hyperledger.besu.tests.acceptance.dsl.privacy.transaction;

import static org.assertj.core.api.Assertions.assertThat;

import org.hyperledger.besu.tests.acceptance.dsl.transaction.NodeRequests;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.Transaction;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.privacy.PrivacyRequestFactory;
import org.hyperledger.besu.tests.acceptance.dsl.transaction.privacy.PrivacyRequestFactory.PrivxFindPrivacyGroupResponse;

import java.io.IOException;
import java.util.List;
Expand All @@ -35,7 +38,13 @@ public FindOnChainPrivacyGroupTransaction(final List<String> nodeEnclaveKeys) {
@Override
public List<PrivacyRequestFactory.OnChainPrivacyGroup> execute(final NodeRequests node) {
try {
return node.privacy().privxFindOnChainPrivacyGroup(nodes).send().getGroups();
PrivxFindPrivacyGroupResponse result =
node.privacy().privxFindOnChainPrivacyGroup(nodes).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getGroups();
} catch (final IOException e) {
throw new RuntimeException(e);
}
Expand Down
Expand Up @@ -33,6 +33,9 @@ public List<String> execute(final NodeRequests node) {
try {
final EthAccounts result = node.eth().ethAccounts().send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getAccounts();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -32,6 +32,9 @@ public String[] execute(final NodeRequests node) {
try {
final EthGetWork result = node.eth().ethGetWork().send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return new String[] {
result.getCurrentBlockHeaderPowHash(),
result.getSeedHashForDag(),
Expand Down
Expand Up @@ -36,8 +36,10 @@ public String execute(final NodeRequests node) {
try {
EthSendTransaction response = node.eth().ethSendRawTransaction(transactionData).send();
assertThat(response).isNotNull();
assertThat(response.hasError()).isTrue();
return response.getError().getMessage();
if (response.hasError()) {
throw new RuntimeException(response.getError().getMessage());
}
return response.getTransactionHash();
} catch (final IOException e) {
throw new RuntimeException(e);
}
Expand Down
Expand Up @@ -33,6 +33,9 @@ public BigInteger execute(final NodeRequests node) {
try {
final NetPeerCount result = node.net().netPeerCount().send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
assertThat(result.hasError()).isFalse();
return result.getQuantity();
} catch (final IOException e) {
Expand Down
Expand Up @@ -36,6 +36,9 @@ public String execute(final NodeRequests node) {
final PermissioningJsonRpcRequestFactory.AddNodeResponse result =
node.perm().addNodesToWhitelist(enodeList).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getResult();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -36,6 +36,9 @@ public Hash execute(final NodeRequests node) {
final PrivacyRequestFactory.SendRawTransactionResponse result =
node.privacy().eeaSendRawTransaction(transaction).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getResult();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -35,6 +35,9 @@ public String execute(final NodeRequests node) {
final PrivacyRequestFactory.DeletePrivacyGroupResponse result =
node.privacy().privDeletePrivacyGroup(transactionHash).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getResult();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -35,6 +35,9 @@ public String execute(final NodeRequests node) {
final PrivacyRequestFactory.PrivDistributeTransactionResponse result =
node.privacy().privDistributeTransaction(transaction).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getTransactionKey();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -36,6 +36,9 @@ public PrivacyGroup[] execute(final NodeRequests node) {
final PrivacyRequestFactory.FindPrivacyGroupResponse result =
node.privacy().privFindPrivacyGroup(groupMembers).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getResult();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -36,6 +36,9 @@ public Integer execute(final NodeRequests node) {
final PrivacyRequestFactory.GetTransactionCountResponse result =
node.privacy().privGetEeaTransactionCount(params).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getCount();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -36,6 +36,9 @@ public Integer execute(final NodeRequests node) {
final PrivacyRequestFactory.GetTransactionCountResponse result =
node.privacy().privGetTransactionCount(params).send();
assertThat(result).isNotNull();
if (result.hasError()) {
throw new RuntimeException(result.getError().getMessage());
}
return result.getCount();
} catch (final IOException e) {
throw new RuntimeException(e);
Expand Down
Expand Up @@ -86,7 +86,7 @@ public static class GetTransactionCountResponse extends Response<Integer> {

@JsonCreator
public GetTransactionCountResponse(@JsonProperty("result") final String result) {
this.count = Integer.decode(result);
this.count = result == null ? null : Integer.decode(result);
}

public Integer getCount() {
Expand Down
Expand Up @@ -21,7 +21,6 @@
import org.hyperledger.besu.tests.acceptance.dsl.node.cluster.ClusterConfigurationBuilder;

import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

public class P2pDisabledAcceptanceTest extends AcceptanceTestBase {
Expand Down Expand Up @@ -54,7 +53,6 @@ public void shouldSucceedExecutingUnaffectedJsonRpcCall() {
}

@Test
@Ignore("Web3J is broken by PR #1426")
public void shouldFailExecutingAffectedJsonRpcCall() {
node.verify(net.awaitPeerCountExceptional());
}
Expand Down
Expand Up @@ -23,7 +23,6 @@

import org.java_websocket.exceptions.WebsocketNotConnectedException;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

public class RpcApisTogglesAcceptanceTest extends AcceptanceTestBase {
Expand Down Expand Up @@ -74,9 +73,8 @@ public void shouldSucceedCallingMethodFromEnabledApiGroup() {
}

@Test
@Ignore("Web3J is broken by PR #1426")
public void shouldFailCallingMethodFromDisabledApiGroup() {
final String expectedMessage = "Invalid response received: 400";
final String expectedMessage = "Method not enabled";

ethApiDisabledNode.verify(eth.accountsExceptional(expectedMessage));
}
Expand Down
Expand Up @@ -40,7 +40,6 @@ public void shouldReturnSuccessResponseWhenMining() {
}

@Test
@Ignore("Web3J is broken by PR #1426")
public void shouldReturnErrorResponseWhenNotMining() {
fullNode.verify(eth.getWorkExceptional("No mining work available yet"));
}
Expand Down
Expand Up @@ -27,9 +27,7 @@

import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.web3j.protocol.exceptions.ClientConnectionException;

public class AllowlistPersistorAcceptanceTest extends AcceptanceTestBase {

Expand Down Expand Up @@ -105,11 +103,10 @@ public void manipulatedNodesWhitelistIsPersisted() {
ALLOWLIST_TYPE.NODES, tempFile, ENODE_TWO, ENODE_ONE, ENODE_THREE));
}

@Ignore("Web3J is broken by PR #1426")
@Test
public void manipulatedNodesWhitelistWithHostnameShouldNotWorkWhenDnsDisabled() {
Assertions.assertThatThrownBy(() -> node.verify(perm.addNodesToAllowlist(ENODE_FOURTH)))
.isInstanceOf(ClientConnectionException.class)
.isInstanceOf(RuntimeException.class)
.hasMessageContaining("Request contains an invalid node");
}
}