[NC-2119] exception handling

pantheon/src/main/java/tech/pegasys/pantheon/PermissioningConfigurationBuilder.java

@@ -18,26 +18,26 @@
 import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.net.URI;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
 import com.google.common.io.Resources;
 import net.consensys.cava.toml.TomlArray;
 import net.consensys.cava.toml.TomlParseResult;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 
 public class PermissioningConfigurationBuilder {
 
-  private static final Logger LOG = LogManager.getLogger();
+  public static final String ACCOUNTS_WHITELIST = "accounts-whitelist";
+  public static final String NODES_WHITELIST = "nodes-whitelist";
 
   // will be used to reload the config from a file while node is running
   public static PermissioningConfiguration permissioningConfigurationFromToml(
       final String configFilePath,
       final boolean permissionedNodeEnabled,
-      final boolean permissionedAccountEnabled) {
+      final boolean permissionedAccountEnabled)
+      throws Exception {
 
     return permissioningConfiguration(
         configFilePath, permissionedNodeEnabled, permissionedAccountEnabled);
@@ -46,14 +46,23 @@ public static PermissioningConfiguration permissioningConfigurationFromToml(
   public static PermissioningConfiguration permissioningConfiguration(
       final String configFilePath,
       final boolean permissionedNodeEnabled,
-      final boolean permissionedAccountEnabled) {
+      final boolean permissionedAccountEnabled)
+      throws Exception {
 
-    TomlParseResult permToml =
-        TomlConfigFileParser.loadConfiguration(
-            permissioningConfigTomlAsString(permissioningConfigFile(configFilePath)));
+    TomlParseResult permToml;
+    boolean foundValidOptions = false;
 
-    TomlArray accountWhitelistTomlArray = permToml.getArray("accounts-whitelist");
-    TomlArray nodeWhitelistTomlArray = permToml.getArray("nodes-whitelist");
+    try {
+      permToml =
+          TomlConfigFileParser.loadConfiguration(
+              permissioningConfigTomlAsString(permissioningConfigFile(configFilePath)));
+    } catch (Exception e) {
+      throw new Exception(
+          "Unable to read permissions TOML config file : " + configFilePath + " " + e.getMessage());
+    }
+
+    TomlArray accountWhitelistTomlArray = permToml.getArray(ACCOUNTS_WHITELIST);
+    TomlArray nodeWhitelistTomlArray = permToml.getArray(NODES_WHITELIST);
 
     final PermissioningConfiguration permissioningConfiguration =
         PermissioningConfiguration.createDefault();
@@ -68,7 +77,8 @@ public static PermissioningConfiguration permissioningConfiguration(
                 .collect(Collectors.toList());
         permissioningConfiguration.setAccountWhitelist(accountsWhitelistToml);
       } else {
-        permissioningConfiguration.setAccountWhitelist(new ArrayList<>());
+        throw new Exception(
+            ACCOUNTS_WHITELIST + " config option missing in TOML config file " + configFilePath);
       }
     }
     if (permissionedNodeEnabled) {
@@ -82,29 +92,25 @@ public static PermissioningConfiguration permissioningConfiguration(
                 .collect(Collectors.toList());
         permissioningConfiguration.setNodeWhitelist(nodesWhitelistToml);
       } else {
-        permissioningConfiguration.setNodeWhitelist(new ArrayList<>());
+        throw new Exception(
+            NODES_WHITELIST + " config option missing in TOML config file " + configFilePath);
       }
     }
     return permissioningConfiguration;
   }
 
-  private static String permissioningConfigTomlAsString(final File file) {
-    try {
-      return Resources.toString(file.toURI().toURL(), UTF_8);
-    } catch (final Exception e) {
-      LOG.error("Unable to load permissioning config {}.", file, e);
-      return null;
-    }
+  private static String permissioningConfigTomlAsString(final File file) throws Exception {
+    return Resources.toString(file.toURI().toURL(), UTF_8);
   }
 
-  private static File permissioningConfigFile(final String filename) {
+  private static File permissioningConfigFile(final String filename) throws FileNotFoundException {
 
     final File permissioningConfigFile = new File(filename);
     if (permissioningConfigFile.exists()) {
       return permissioningConfigFile;
     } else {
-      LOG.error("File does not exist: permissioning config path: {}.", filename);
-      return null;
+      throw new FileNotFoundException(
+          "File does not exist: permissioning config path: " + filename);
     }
   }
 }

pantheon/src/main/java/tech/pegasys/pantheon/TomlConfigFileParser.java

@@ -17,29 +17,25 @@
 import net.consensys.cava.toml.Toml;
 import net.consensys.cava.toml.TomlParseError;
 import net.consensys.cava.toml.TomlParseResult;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 
 public class TomlConfigFileParser {
 
-  private static final Logger LOG = LogManager.getLogger();
-
   private static TomlParseResult checkConfigurationValidity(
-      final TomlParseResult result, final String toml) {
+      final TomlParseResult result, final String toml) throws Exception {
     if (result == null || result.isEmpty()) {
-      LOG.error("Unable to read TOML configuration file %s", toml);
+      throw new Exception("Empty TOML result: " + toml);
     }
     return result;
   }
 
-  public static TomlParseResult loadConfiguration(final String toml) {
+  public static TomlParseResult loadConfiguration(final String toml) throws Exception {
     TomlParseResult result = Toml.parse(toml);
 
     if (result.hasErrors()) {
       final String errors =
           result.errors().stream().map(TomlParseError::toString).collect(Collectors.joining("%n"));
       ;
-      LOG.error("Invalid TOML configuration : %s", errors);
+      throw new Exception("Invalid TOML configuration : " + errors);
     }
 
     return checkConfigurationValidity(result, toml);

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -583,23 +583,27 @@ public void run() {
     }
 
     final EthNetworkConfig ethNetworkConfig = updateNetworkConfig(getNetwork());
-    final Optional<PermissioningConfiguration> permissioningConfiguration =
-        permissioningConfiguration();
-    permissioningConfiguration.ifPresent(
-        p -> ensureAllBootnodesAreInWhitelist(ethNetworkConfig, p));
-
-    synchronize(
-        buildController(),
-        p2pEnabled,
-        peerDiscoveryEnabled,
-        ethNetworkConfig.getBootNodes(),
-        maxPeers,
-        p2pHost,
-        p2pPort,
-        jsonRpcConfiguration(),
-        webSocketConfiguration(),
-        metricsConfiguration(),
-        permissioningConfiguration);
+    try {
+      final Optional<PermissioningConfiguration> permissioningConfiguration =
+          permissioningConfiguration();
+      permissioningConfiguration.ifPresent(
+          p -> ensureAllBootnodesAreInWhitelist(ethNetworkConfig, p));
+
+      synchronize(
+          buildController(),
+          p2pEnabled,
+          peerDiscoveryEnabled,
+          ethNetworkConfig.getBootNodes(),
+          maxPeers,
+          p2pHost,
+          p2pPort,
+          jsonRpcConfiguration(),
+          webSocketConfiguration(),
+          metricsConfiguration(),
+          permissioningConfiguration);
+    } catch (Exception e) {
+      throw new ParameterException(new CommandLine(this), e.getMessage());
+    }
   }
 
   private NetworkName getNetwork() {
@@ -734,7 +738,7 @@ MetricsConfiguration metricsConfiguration() {
     return metricsConfiguration;
   }
 
-  private Optional<PermissioningConfiguration> permissioningConfiguration() {
+  private Optional<PermissioningConfiguration> permissioningConfiguration() throws Exception {
 
     if (!permissionsAccountsEnabled && !permissionsNodesEnabled) {
       return Optional.empty();

pantheon/src/test/java/tech/pegasys/pantheon/PermissioningConfigurationBuilderTest.java

@@ -17,7 +17,6 @@
 
 import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration;
 
-import java.io.IOException;
 import java.net.URI;
 import java.net.URL;
 import java.nio.file.Files;
@@ -28,7 +27,7 @@
 
 public class PermissioningConfigurationBuilderTest {
 
-  static final String PERMISSIONING_CONFIG_TOML = "permissioning_config.toml";
+  static final String PERMISSIONING_CONFIG_VALID = "permissioning_config.toml";
   static final String PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY =
       "permissioning_config_account_whitelist_only.toml";
   static final String PERMISSIONING_CONFIG_NODE_WHITELIST_ONLY =
@@ -39,17 +38,19 @@ public class PermissioningConfigurationBuilderTest {
       "permissioning_config_empty_whitelists.toml";
   static final String PERMISSIONING_CONFIG_ABSENT_WHITELISTS =
       "permissioning_config_absent_whitelists.toml";
+  static final String PERMISSIONING_CONFIG_UNRECOGNIZED_KEY =
+      "permissioning_config_unrecognized_key.toml";
 
   private final String VALID_NODE_ID =
       "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0";
 
   @Test
-  public void permissioningConfig() throws IOException {
+  public void permissioningConfig() throws Exception {
 
     final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567";
     final String uri2 = "enode://" + VALID_NODE_ID + "@192.169.0.9:4568";
 
-    final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_TOML);
+    final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID);
     final Path toml = Files.createTempFile("toml", "");
     Files.write(toml, Resources.toByteArray(configFile));
 
@@ -66,7 +67,7 @@ public void permissioningConfig() throws IOException {
   }
 
   @Test
-  public void permissioningConfigWithOnlyNodeWhitelistSet() throws IOException {
+  public void permissioningConfigWithOnlyNodeWhitelistSet() throws Exception {
 
     final String uri = "enode://" + VALID_NODE_ID + "@192.168.0.9:4567";
 
@@ -84,7 +85,7 @@ public void permissioningConfigWithOnlyNodeWhitelistSet() throws IOException {
   }
 
   @Test
-  public void permissioningConfigWithOnlyAccountWhitelistSet() throws IOException {
+  public void permissioningConfigWithOnlyAccountWhitelistSet() throws Exception {
 
     final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ACCOUNT_WHITELIST_ONLY);
     final Path toml = Files.createTempFile("toml", "");
@@ -101,7 +102,7 @@ public void permissioningConfigWithOnlyAccountWhitelistSet() throws IOException
   }
 
   @Test
-  public void permissioningConfigWithInvalidEnode() throws IOException {
+  public void permissioningConfigWithInvalidEnode() throws Exception {
 
     final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_INVALID_ENODE);
     final Path toml = Files.createTempFile("toml", "");
@@ -117,7 +118,7 @@ public void permissioningConfigWithInvalidEnode() throws IOException {
   }
 
   @Test
-  public void permissioningConfigWithEmptyWhitelistMustNotError() throws IOException {
+  public void permissioningConfigWithEmptyWhitelistMustNotError() throws Exception {
 
     final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_EMPTY_WHITELISTS);
     final Path toml = Files.createTempFile("toml", "");
@@ -134,26 +135,59 @@ public void permissioningConfigWithEmptyWhitelistMustNotError() throws IOExcepti
   }
 
   @Test
-  public void permissioningConfigWithAbsentWhitelistMustSetValues() throws IOException {
+  public void permissioningConfigWithAbsentWhitelistMustThrowException() throws Exception {
 
     final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS);
     final Path toml = Files.createTempFile("toml", "");
     Files.write(toml, Resources.toByteArray(configFile));
 
-    PermissioningConfiguration permissioningConfiguration =
-        PermissioningConfigurationBuilder.permissioningConfiguration(
-            toml.toAbsolutePath().toString(), true, true);
+    try {
+      PermissioningConfigurationBuilder.permissioningConfiguration(
+          toml.toAbsolutePath().toString(), true, true);
+      fail("expected exception: no valid whitelists in the TOML file");
+    } catch (Exception e) {
+      assertThat(e.getMessage().contains("Unexpected end of line")).isTrue();
+    }
+  }
 
-    assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue();
-    assertThat(permissioningConfiguration.getNodeWhitelist()).isEmpty();
-    assertThat(permissioningConfiguration.isAccountWhitelistEnabled()).isTrue();
-    assertThat(permissioningConfiguration.getAccountWhitelist()).isEmpty();
+  @Test
+  public void permissioningConfigWithUnrecognizedKeyMustThrowException() throws Exception {
+
+    final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_UNRECOGNIZED_KEY);
+    final Path toml = Files.createTempFile("toml", "");
+    Files.write(toml, Resources.toByteArray(configFile));
+
+    try {
+      PermissioningConfigurationBuilder.permissioningConfiguration(
+          toml.toAbsolutePath().toString(), true, true);
+      fail("expected exception: didn't find a recognized key in the TOML file");
+    } catch (Exception e) {
+      assertThat(e.getMessage().contains("config option missing")).isTrue();
+      assertThat(e.getMessage().contains(PermissioningConfigurationBuilder.ACCOUNTS_WHITELIST))
+          .isTrue();
+    }
   }
 
   @Test
-  public void permissioningConfigFromFileMustSetFilePath() throws IOException {
+  public void permissioningConfigWithEmptyFileMustThrowException() throws Exception {
 
-    final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_ABSENT_WHITELISTS);
+    // write an empty file
+    final Path toml = Files.createTempFile("toml", "");
+
+    try {
+      PermissioningConfigurationBuilder.permissioningConfiguration(
+          toml.toAbsolutePath().toString(), true, true);
+      fail("expected exception: empty TOML file");
+
+    } catch (Exception e) {
+      assertThat(e.getMessage().contains("Empty TOML result")).isTrue();
+    }
+  }
+
+  @Test
+  public void permissioningConfigFromFileMustSetFilePath() throws Exception {
+
+    final URL configFile = Resources.getResource(PERMISSIONING_CONFIG_VALID);
     final Path toml = Files.createTempFile("toml", "");
     Files.write(toml, Resources.toByteArray(configFile));
 
@@ -162,10 +196,17 @@ public void permissioningConfigFromFileMustSetFilePath() throws IOException {
             toml.toString(), true, true);
 
     assertThat(permissioningConfiguration.getConfigurationFilePath()).isEqualTo(toml.toString());
+  }
 
-    assertThat(permissioningConfiguration.isNodeWhitelistEnabled()).isTrue();
-    assertThat(permissioningConfiguration.getNodeWhitelist()).isEmpty();
-    assertThat(permissioningConfiguration.isAccountWhitelistEnabled()).isTrue();
-    assertThat(permissioningConfiguration.getAccountWhitelist()).isEmpty();
+  @Test
+  public void permissioningConfigFromNonexistentFileMustThrowException() {
+
+    try {
+      PermissioningConfigurationBuilder.permissioningConfigurationFromToml(
+          "file-does-not-exist", true, true);
+      fail("expected exception: file does not exist");
+    } catch (Exception e) {
+      assertThat(e.getMessage().contains("File does not exist")).isTrue();
+    }
   }
 }

pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java

@@ -320,6 +320,17 @@ public void permissionsTomlPathWithoutOptionMustDisplayUsage() {
     assertThat(commandOutput.toString()).isEmpty();
   }
 
+  @Test
+  public void permissionsEnabledWithNonexistentConfigFileMustError() {
+    parseCommand(
+        "--permissions-accounts-enabled", "--permissions-config-path", "file-does-not-exist");
+
+    verifyZeroInteractions(mockRunnerBuilder);
+
+    assertThat(commandErrorOutput.toString()).contains("File does not exist");
+    assertThat(commandOutput.toString()).isEmpty();
+  }
+
   @Test
   public void permissionsTomlPathMustUseOption() throws IOException {
 

pantheon/src/test/resources/permissioning_config_unrecognized_key.toml

@@ -0,0 +1,3 @@
+# Permissioning TOML file with typo in key
+
+perm-node-whitelist=[]