Merge pull request #824 from corda/kat/doorman-playing

ENT-1872 - Better cmdline failure msgs for Doorman

network-management/build.gradle

@@ -81,7 +81,7 @@ dependencies {
     compile "org.glassfish.jersey.containers:jersey-container-jetty-http:${jersey_version}"
 
     // JOpt: for command line flags.
-    compile "net.sf.jopt-simple:jopt-simple:5.0.2"
+    compile "net.sf.jopt-simple:jopt-simple:5.0.4"
 
     // TypeSafe Config: for simple and human friendly config files.
     compile "com.typesafe:config:$typesafe_config_version"

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/utils/ArgsParser.kt

@@ -22,15 +22,30 @@ abstract class ArgsParser<out T : Any> {
             printHelpOn?.let {
                 it.println(e.message ?: "Unable to parse arguments.")
                 optionParser.printHelpOn(it)
-                exitProcess(-1)
+                exitProcess(1)
             }
             throw e
         }
+
         if (optionSet.has(helpOption)) {
             printHelpOn?.let(optionParser::printHelpOn)
             exitProcess(0)
         }
-        return parse(optionSet)
+
+        return try {
+            parse(optionSet)
+        } catch (e: RuntimeException) {
+            // We handle errors from the parsing of the command line arguments as a runtime
+            // exception because the joptsimple library is overly helpful and doesn't expose
+            // parsing / conversion exceptions in a way that makes reporting the message out
+            // to the user possible. Thus, that library is re-throwing those exceptions as simple
+            // runtime exceptions with a modified cause to preserve the error location
+            printHelpOn?.let {
+                it.println("ERROR: ${e.message ?: "Unable to parse arguments."}")
+                exitProcess(2)
+            }
+            throw e
+        }
     }
 
     protected abstract fun parse(optionSet: OptionSet): T

network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/DoormanArgsParser.kt

@@ -2,6 +2,7 @@ package com.r3.corda.networkmanage.doorman
 
 import com.google.common.primitives.Booleans
 import com.r3.corda.networkmanage.common.utils.ArgsParser
+import joptsimple.OptionException
 import joptsimple.OptionSet
 import joptsimple.util.EnumConverter
 import joptsimple.util.PathConverter
@@ -34,8 +35,24 @@ class DoormanArgsParser : ArgsParser<DoormanCmdLineOptions>() {
             .withRequiredArg()
 
     override fun parse(optionSet: OptionSet): DoormanCmdLineOptions {
-        val configFile = optionSet.valueOf(configFileArg)
-        val mode = optionSet.valueOf(modeArg)
+        val configFile = try {
+            optionSet.valueOf(configFileArg)
+        } catch (e: OptionException) {
+            throw RuntimeException("Specified config file doesn't exist").apply {
+                initCause(e.cause)
+            }
+        }
+
+        val mode = try {
+            optionSet.valueOf(modeArg)
+        } catch (e: OptionException) {
+            throw RuntimeException(
+                    "Unknown mode specified, valid options are [${Mode.values().joinToString(", ")}]"
+            ).apply {
+                initCause(e.cause)
+            }
+        }
+
         val setNetworkParametersFile = optionSet.valueOf(setNetworkParametersArg)
         val flagDay = optionSet.has(flagDayArg)
         val cancelUpdate = optionSet.has(cancelUpdateArg)

network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/Main.kt

@@ -30,7 +30,7 @@ fun main(args: Array<String>) {
     }
 
     initialiseSerialization()
-    val cmdLineOptions = DoormanArgsParser().parseOrExit(*args)
+    val cmdLineOptions = DoormanArgsParser().parseOrExit(*args, printHelpOn = System.err)
 
     val config = parseConfig<NetworkManagementServerConfig>(cmdLineOptions.configFile)
 

network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/DoormanArgsParserTest.kt

@@ -25,15 +25,16 @@ class DoormanArgsParserTest {
     @Test
     fun `should fail when network parameters file is missing`() {
         assertThatThrownBy {
-            argsParser.parseOrExit("--config-file", validConfigPath, "--set-network-parameters", "not-here")
+            argsParser.parseOrExit("--config-file", validConfigPath, "--set-network-parameters", "not-here",
+                    printHelpOn = null)
         }.hasMessageContaining("not-here")
     }
 
     @Test
     fun `should fail when config file is missing`() {
         assertThatThrownBy {
-            argsParser.parseOrExit("--config-file", "not-existing-file")
-        }.hasMessageContaining("not-existing-file")
+            argsParser.parseOrExit("--config-file", "not-existing-file", printHelpOn = null)
+        }.hasMessageContaining("Specified config file doesn't exist")
     }
 
     @Test
@@ -41,16 +42,16 @@ class DoormanArgsParserTest {
         val parameter = argsParser.parseOrExit("--config-file", validConfigPath, "--mode", "ROOT_KEYGEN", "--trust-store-password", "testPassword")
         assertEquals("testPassword", parameter.trustStorePassword)
 
-        assertFailsWith<OptionException> {
+        assertFailsWith<RuntimeException> {
             argsParser.parseOrExit("--trust-store-password", printHelpOn = null)
         }
 
         // Should fail if password is provided in mode other then root keygen.
         assertFailsWith<IllegalArgumentException> {
-            argsParser.parseOrExit("--config-file", validConfigPath, "--trust-store-password", "testPassword")
+            argsParser.parseOrExit("--config-file", validConfigPath, "--trust-store-password", "testPassword", printHelpOn = null)
         }
 
         // trust store password is optional.
-        assertNull(argsParser.parseOrExit("--config-file", validConfigPath).trustStorePassword)
+        assertNull(argsParser.parseOrExit("--config-file", validConfigPath, printHelpOn = null).trustStorePassword)
     }
 }

network-management/src/test/kotlin/com/r3/corda/networkmanage/hsm/configuration/SigningServiceArgsParserTest.kt

@@ -51,7 +51,7 @@ class SigningServiceArgsParserTest : TestBase() {
     @Test
     fun `should fail when config file is missing`() {
         assertThatThrownBy {
-            argsParser.parseOrExit("--config-file", "not-existing-file")
+            argsParser.parseOrExit("--config-file", "not-existing-file", printHelpOn = null)
         }.hasMessageContaining("not-existing-file")
     }
 }

network-management/src/test/kotlin/com/r3/corda/networkmanage/hsm/generator/GeneratorParametersTest.kt

@@ -30,7 +30,7 @@ class GeneratorParametersTest {
     @Test
     fun `should fail when config file is missing`() {
         val message = assertFailsWith<OptionException> {
-            ConfigFilePathArgsParser().parseOrExit("--config-file", "not-existing-file")
+            ConfigFilePathArgsParser().parseOrExit("--config-file", "not-existing-file", printHelpOn = null)
         }.message
         Assertions.assertThat(message).contains("not-existing-file")
     }