Cli error message improvements (#172)

* Errors in Generate/Validate print to stderr/exit 1

Generate and Validate exposed exceptions rather than user-friendly
messages when an error occurred. In generate, this could happen for
numerous reasons, but the most likely is a user typing (or guessing) an
invalid generator name. In Validate, an error was exposed if there were
any validation errors in a spec.

New behavior:

* Generate now exposes a typed exception when a generator cannot be
  loaded by name. This allows consistent messaging for load failures.
* Generate now presents guidance on failure (check the spelling and try
  again). This is purely a usability improvement.
* Validate now writes validation errors to stderr and exits with code 1.

* Improve err messages: config-help/required opts.

config-help now presents same error for invalid generator names as the
'generate' command.

Options which are required, and those which require a value, now present
a user-friendly hint at the error and exit with code 1 (rather than an
uncaught exception).

* Log missing -g error to stderr rather than LOGGER

Author: jimschubert

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/OpenAPIGenerator.java

@@ -19,6 +19,8 @@
 
 import io.airlift.airline.Cli;
 import io.airlift.airline.Help;
+import io.airlift.airline.ParseOptionMissingException;
+import io.airlift.airline.ParseOptionMissingValueException;
 import org.openapitools.codegen.cmd.*;
 
 import java.util.Arrays;
@@ -53,8 +55,6 @@ public static void main(String[] args) {
                                 Version.class
                         );
 
-        builder.build().parse(args).run();
-
         // If CLI is run without a command, consider this an error.
         // We can check against empty args because unrecognized arguments/commands result in an exception.
         // This is useful to exit with status 1, for example, so that misconfigured scripts fail fast.
@@ -64,5 +64,12 @@ public static void main(String[] args) {
         if (args.length == 0) {
             System.exit(1);
         }
+
+        try {
+            builder.build().parse(args).run();
+        } catch (ParseOptionMissingException | ParseOptionMissingValueException e) {
+            System.err.printf("[error] %s%n", e.getMessage());
+            System.exit(1);
+        }
     }
 }

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ConfigHelp.java

@@ -22,6 +22,7 @@
 import org.openapitools.codegen.CliOption;
 import org.openapitools.codegen.CodegenConfig;
 import org.openapitools.codegen.CodegenConfigLoader;
+import org.openapitools.codegen.GeneratorNotFoundException;
 
 @Command(name = "config-help", description = "Config help for chosen lang")
 public class ConfigHelp implements Runnable {
@@ -32,14 +33,20 @@ public class ConfigHelp implements Runnable {
 
     @Override
     public void run() {
-        System.out.println();
-        CodegenConfig config = CodegenConfigLoader.forName(lang);
-        System.out.println("CONFIG OPTIONS");
-        for (CliOption langCliOption : config.cliOptions()) {
-            System.out.println("\t" + langCliOption.getOpt());
-            System.out.println("\t    "
-                    + langCliOption.getOptionHelp().replaceAll("\n", "\n\t    "));
+        try {
+            CodegenConfig config = CodegenConfigLoader.forName(lang);
             System.out.println();
+            System.out.println("CONFIG OPTIONS");
+            for (CliOption langCliOption : config.cliOptions()) {
+                System.out.println("\t" + langCliOption.getOpt());
+                System.out.println("\t    "
+                        + langCliOption.getOptionHelp().replaceAll("\n", System.lineSeparator() + "\t    "));
+                System.out.println();
+            }
+        } catch (GeneratorNotFoundException e) {
+            System.err.println(e.getMessage());
+            System.err.println("[error] Check the spelling of the generator's name and try again.");
+            System.exit(1);
         }
     }
 }

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Generate.java

@@ -22,11 +22,11 @@
 import org.openapitools.codegen.ClientOptInput;
 import org.openapitools.codegen.CodegenConstants;
 import org.openapitools.codegen.DefaultGenerator;
+import org.openapitools.codegen.GeneratorNotFoundException;
 import org.openapitools.codegen.config.CodegenConfigurator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.commons.lang3.StringUtils.isEmpty;
 import static org.openapitools.codegen.config.CodegenConfiguratorUtils.*;
 import static org.apache.commons.lang3.StringUtils.isNotEmpty;
 
@@ -226,7 +226,7 @@ public void run() {
             LOGGER.warn("The '--lang' and '-l' are deprecated and may reference language names only in the next major release (4.0). Please use --generator-name /-g instead.");
             configurator.setGeneratorName(lang);
         } else {
-            LOGGER.error("A generator name (--generator-name / -g) is required. ");
+            System.err.println("[error] A generator name (--generator-name / -g) is required.");
             System.exit(1);
         }
 
@@ -309,8 +309,14 @@ public void run() {
         applyAdditionalPropertiesKvpList(additionalProperties, configurator);
         applyLanguageSpecificPrimitivesCsvList(languageSpecificPrimitives, configurator);
         applyReservedWordsMappingsKvpList(reservedWordsMappings, configurator);
-        final ClientOptInput clientOptInput = configurator.toClientOptInput();
 
-        new DefaultGenerator().opts(clientOptInput).generate();
+        try {
+            final ClientOptInput clientOptInput = configurator.toClientOptInput();
+            new DefaultGenerator().opts(clientOptInput).generate();
+        } catch (GeneratorNotFoundException e) {
+            System.err.println(e.getMessage());
+            System.err.println("[error] Check the spelling of the generator's name and try again.");
+            System.exit(1);
+        }
     }
 }

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/Validate.java

@@ -36,18 +36,24 @@ public class Validate implements Runnable {
 
     @Override
     public void run() {
-        System.out.println("Validating spec file (" + spec + ")");
+        System.out.println("Validating spec (" + spec + ")");
 
         SwaggerParseResult result = new OpenAPIParser().readLocation(spec, null, null);
         List<String> messageList = result.getMessages();
         Set<String> messages = new HashSet<String>(messageList);
 
-        for (String message : messages) {
-            System.out.println(message);
-        }
-
         if (messages.size() > 0) {
-            throw new ValidateException();
+            StringBuilder sb = new StringBuilder();
+            sb.append(System.lineSeparator());
+            for (String message : messages) {
+                sb.append(String.format("\t- %s%s", message, System.lineSeparator()));
+            }
+            sb.append(System.lineSeparator());
+            sb.append("[error] Spec is invalid.");
+            System.err.println(sb.toString());
+            System.exit(1);
+        } else {
+            System.out.println("No validation errors detected.");
         }
     }
 }

modules/openapi-generator-cli/src/main/java/org/openapitools/codegen/cmd/ValidateException.java

@@ -47,7 +47,7 @@ public static CodegenConfig forName(String name) {
         try {
             return (CodegenConfig) Class.forName(name).newInstance();
         } catch (Exception e) {
-            throw new RuntimeException("Can't load config class with name '".concat(name) + "'\nAvailable:\n" + availableConfigs.toString());
+            throw new GeneratorNotFoundException("Can't load config class with name '".concat(name) + "'\nAvailable:\n" + availableConfigs.toString(), e);
         }
     }
 

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenConfigLoader.java

@@ -0,0 +1,80 @@
+package org.openapitools.codegen;
+
+/**
+ * Typed exception exposing issues with loading generators (e.g. by name).
+ */
+@SuppressWarnings("unused")
+public class GeneratorNotFoundException extends RuntimeException {
+    /**
+     * Constructs a new runtime exception with {@code null} as its
+     * detail message.  The cause is not initialized, and may subsequently be
+     * initialized by a call to {@link #initCause}.
+     */
+    public GeneratorNotFoundException() {
+    }
+
+    /**
+     * Constructs a new runtime exception with the specified detail message.
+     * The cause is not initialized, and may subsequently be initialized by a
+     * call to {@link #initCause}.
+     *
+     * @param message the detail message. The detail message is saved for
+     *                later retrieval by the {@link #getMessage()} method.
+     */
+    public GeneratorNotFoundException(String message) {
+        super(message);
+    }
+
+    /**
+     * Constructs a new runtime exception with the specified detail message and
+     * cause.  <p>Note that the detail message associated with
+     * {@code cause} is <i>not</i> automatically incorporated in
+     * this runtime exception's detail message.
+     *
+     * @param message the detail message (which is saved for later retrieval
+     *                by the {@link #getMessage()} method).
+     * @param cause   the cause (which is saved for later retrieval by the
+     *                {@link #getCause()} method).  (A <tt>null</tt> value is
+     *                permitted, and indicates that the cause is nonexistent or
+     *                unknown.)
+     * @since 1.4
+     */
+    public GeneratorNotFoundException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    /**
+     * Constructs a new runtime exception with the specified cause and a
+     * detail message of <tt>(cause==null ? null : cause.toString())</tt>
+     * (which typically contains the class and detail message of
+     * <tt>cause</tt>).  This constructor is useful for runtime exceptions
+     * that are little more than wrappers for other throwables.
+     *
+     * @param cause the cause (which is saved for later retrieval by the
+     *              {@link #getCause()} method).  (A <tt>null</tt> value is
+     *              permitted, and indicates that the cause is nonexistent or
+     *              unknown.)
+     * @since 1.4
+     */
+    public GeneratorNotFoundException(Throwable cause) {
+        super(cause);
+    }
+
+    /**
+     * Constructs a new runtime exception with the specified detail
+     * message, cause, suppression enabled or disabled, and writable
+     * stack trace enabled or disabled.
+     *
+     * @param message            the detail message.
+     * @param cause              the cause.  (A {@code null} value is permitted,
+     *                           and indicates that the cause is nonexistent or unknown.)
+     * @param enableSuppression  whether or not suppression is enabled
+     *                           or disabled
+     * @param writableStackTrace whether or not the stack trace should
+     *                           be writable
+     * @since 1.7
+     */
+    public GeneratorNotFoundException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
+        super(message, cause, enableSuppression, writableStackTrace);
+    }
+}