Fix JSON-RPC Errors in ATs (#1428)

Turns out it's not really a web3j problem, but an issue with how we read
errors now that we don't throw HTTP status codes for expected failures.
Most of the fixes revolve around having the AT framework checking for an
error and throwing a RuntimeException with the message.  Others involve
a new target exception type, with one strange serialization case.

Signed-off-by: Danno Ferrin <[email protected]>

CHANGELOG.md

@@ -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
 

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/eth/ExpectEthAccountsException.java

@@ -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;
@@ -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);
   }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/eth/ExpectEthGetWorkException.java

@@ -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;
@@ -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);
   }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/eth/ExpectEthSendRawTransactionException.java

@@ -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;
@@ -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);
   }
 }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/condition/priv/ExpectJsonRpcError.java

@@ -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());
     }
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/condition/ExpectInternalErrorPrivateTransactionReceipt.java

@@ -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;
@@ -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");
     }
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/transaction/CreatePrivacyGroupTransaction.java

@@ -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> {
@@ -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);
     }
   }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/privacy/transaction/FindOnChainPrivacyGroupTransaction.java

@@ -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;
@@ -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);
     }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthAccountsTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthGetWorkTransaction.java

@@ -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(),

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthSendRawTransactionTransaction.java

@@ -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);
     }

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/net/NetPeerCountTransaction.java

@@ -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) {

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/perm/PermAddNodeTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/EeaSendRawTransactionTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivDeletePrivacyGroupTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivDistributeRawTransactionTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivFindPrivacyGroupTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivGetEeaTransactionCountTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivGetTransactionCountTransaction.java

@@ -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);

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/privacy/PrivacyRequestFactory.java

@@ -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() {

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/P2pDisabledAcceptanceTest.java

@@ -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 {
@@ -54,7 +53,6 @@ public void shouldSucceedExecutingUnaffectedJsonRpcCall() {
   }
 
   @Test
-  @Ignore("Web3J is broken by PR #1426")
   public void shouldFailExecutingAffectedJsonRpcCall() {
     node.verify(net.awaitPeerCountExceptional());
   }

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/RpcApisTogglesAcceptanceTest.java

@@ -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 {
@@ -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));
   }

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/jsonrpc/EthGetWorkAcceptanceTest.java

@@ -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"));
   }

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/AllowlistPersistorAcceptanceTest.java

@@ -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 {
 
@@ -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");
   }
 }