Skip to content

Commit

Permalink
Cli error message improvements (#172)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jimschubert authored and wing328 committed May 29, 2018
1 parent 5a87fe6 commit f042132
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

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

Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit f042132

Please sign in to comment.