Skip to content

Commit

Permalink
Improve CLI error messages (#3078)
Browse files Browse the repository at this point in the history 
Improve CLI error message by including "caused by"
  • Loading branch information
@nvoxland
nvoxland committed Jul 28, 2022
1 parent 7315bf4 commit 16337ee
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 34 deletions.
51 changes: 35 additions & 16 deletions liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java
View file
Expand Up @@ -211,26 +211,29 @@ private CommandLine buildPicoCommandLine() {
private int handleException(Throwable exception) {
Throwable cause = exception;

String bestMessage = exception.getMessage();
while (cause.getCause() != null) {
if (StringUtil.trimToNull(cause.getMessage()) != null) {
bestMessage = cause.getMessage();
String uiMessage = "";
while (cause != null) {
String newMessage = StringUtil.trimToNull(cleanExceptionMessage(cause.getMessage()));
if (newMessage != null) {
if (!uiMessage.contains(newMessage)) {
if (!uiMessage.equals("")) {
uiMessage += System.lineSeparator() + " - Caused by: ";
}
uiMessage += newMessage;
}
}

cause = cause.getCause();
}

if (bestMessage == null) {
bestMessage = exception.getClass().getName();
} else {
//clean up message
bestMessage = bestMessage.replaceFirst("^[\\w.]*exception[\\w.]*: ", "");
bestMessage = bestMessage.replace("Unexpected error running Liquibase: ", "");
if (StringUtil.isEmpty(uiMessage)) {
uiMessage = exception.getClass().getName();
}

if (cause instanceof CommandFailedException && ((CommandFailedException) cause).isExpected()) {
Scope.getCurrentScope().getLog(getClass()).severe(bestMessage);
Scope.getCurrentScope().getLog(getClass()).severe(uiMessage);
} else {
Scope.getCurrentScope().getLog(getClass()).severe(bestMessage, exception);
Scope.getCurrentScope().getLog(getClass()).severe(uiMessage, exception);
}

boolean printUsage = false;
Expand All @@ -240,20 +243,20 @@ private int handleException(Throwable exception) {
if (exception instanceof CommandLine.UnmatchedArgumentException) {
System.err.println("Unexpected argument(s): " + StringUtil.join(((CommandLine.UnmatchedArgumentException) exception).getUnmatched(), ", "));
} else {
System.err.println("Error parsing command line: " + bestMessage);
System.err.println("Error parsing command line: " + uiMessage);
}
CommandLine.UnmatchedArgumentException.printSuggestions((CommandLine.ParameterException) exception, suggestionsPrintWriter);

printUsage = true;
} else if (exception instanceof IllegalArgumentException
|| exception instanceof CommandValidationException
|| exception instanceof CommandLineParsingException) {
System.err.println("Error parsing command line: " + bestMessage);
System.err.println("Error parsing command line: " + uiMessage);
printUsage = true;
} else if (exception.getCause() != null && exception.getCause() instanceof CommandFailedException) {
System.err.println(bestMessage);
System.err.println(uiMessage);
} else {
System.err.println("\nUnexpected error running Liquibase: " + bestMessage);
System.err.println("\nUnexpected error running Liquibase: " + uiMessage);
System.err.println();

if (Level.OFF.equals(this.configuredLogLevel)) {
Expand Down Expand Up @@ -286,6 +289,22 @@ private int handleException(Throwable exception) {
return 1;
}

protected String cleanExceptionMessage(String message) {
if (message == null) {
return null;
}

String originalMessage;
do {
originalMessage = message;
message = message.replaceFirst("^[\\w.]*Exception: ", "");
message = message.replaceFirst("^[\\w.]*Error: ", "");
} while (!originalMessage.equals(message));

message = message.replace("Unexpected error running Liquibase: ", "");
return message;
}

public int execute(String[] args) {
try {
final String[] finalArgs = adjustLegacyArgs(args);
Expand Down
4 changes: 2 additions & 2 deletions liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java
View file
Expand Up @@ -88,9 +88,9 @@ public static void main(final String[] args) throws Exception {

ClassLoader parentLoader;
if (parentLoaderSetting.equalsIgnoreCase("system")) {
//loading with the regular system classloader includes liquibase.jar in the parent.
//loading with the regular system classloader includes liquibase.jar in the parent.
//That causes the parent classloader to load LiquibaseCommandLine which makes it not able to access files in the child classloader
//The system classloader's parent is the boot classloader, which keeps the only classloader with liquibase-core.jar the same as the rest of the classes it needs to access.
//The system classloader's parent is the boot classloader, which keeps the only classloader with liquibase-core.jar the same as the rest of the classes it needs to access.
parentLoader = ClassLoader.getSystemClassLoader().getParent();

} else if (parentLoaderSetting.equalsIgnoreCase("thread")) {
Expand Down
46 changes: 31 additions & 15 deletions ...ase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy
View file
Expand Up @@ -43,8 +43,8 @@ class LiquibaseCommandLineTest extends Specification {
LiquibaseCommandLine.toArgNames(new ConfigurationDefinition.Builder(prefix).define(argName, String).addAliasKey(alias).buildTemporary()).join(", ") == expected

where:
prefix | argName | alias | expected
"liquibase" | "test" | "testAlias" | "--test, --liquibase-test, --liquibasetest, --test-alias, --testAlias"
prefix | argName | alias | expected
"liquibase" | "test" | "testAlias" | "--test, --liquibase-test, --liquibasetest, --test-alias, --testAlias"
}

@Unroll
Expand All @@ -53,19 +53,19 @@ class LiquibaseCommandLineTest extends Specification {
new LiquibaseCommandLine().adjustLegacyArgs(input as String[]).toArrayString() == (expected as String[]).toArrayString()

where:
input | expected
["--arg", "update", "--help"] | ["--arg", "update", "--help"]
["tag", "--help"] | ["tag", "--help"]
["tag", "my-tag"] | ["tag", "--tag", "my-tag"]
["rollback", "my-tag"] | ["rollback", "--tag", "my-tag"]
["rollbackToDate", "1/2/3"] | ["rollbackToDate", "--date", "1/2/3"]
["rollback-to-date", "1/2/3"] | ["rollback-to-date", "--date", "1/2/3"]
["rollback-to-date", "1/2/3", "3:15:21"] | ["rollback-to-date", "--date", "1/2/3 3:15:21"]
["rollback-count", "5"] | ["rollback-count", "--count", "5"]
["future-rollback-count-sql", "5"] | ["future-rollback-count-sql", "--count", "5"]
["future-rollback-from-tag-sql", "my-tag"] | ["future-rollback-from-tag-sql", "--tag", "my-tag"]

["--log-level","DEBUG","--log-file","06V21.txt","--defaultsFile=liquibase.h2-mem.properties","update","--changelog-file","postgres_lbpro_master_changelog.xml","--labels","setup"] | ["--log-level","DEBUG","--log-file","06V21.txt","--defaultsFile=liquibase.h2-mem.properties","update","--changelog-file","postgres_lbpro_master_changelog.xml","--labels","setup"]
input | expected
["--arg", "update", "--help"] | ["--arg", "update", "--help"]
["tag", "--help"] | ["tag", "--help"]
["tag", "my-tag"] | ["tag", "--tag", "my-tag"]
["rollback", "my-tag"] | ["rollback", "--tag", "my-tag"]
["rollbackToDate", "1/2/3"] | ["rollbackToDate", "--date", "1/2/3"]
["rollback-to-date", "1/2/3"] | ["rollback-to-date", "--date", "1/2/3"]
["rollback-to-date", "1/2/3", "3:15:21"] | ["rollback-to-date", "--date", "1/2/3 3:15:21"]
["rollback-count", "5"] | ["rollback-count", "--count", "5"]
["future-rollback-count-sql", "5"] | ["future-rollback-count-sql", "--count", "5"]
["future-rollback-from-tag-sql", "my-tag"] | ["future-rollback-from-tag-sql", "--tag", "my-tag"]

["--log-level", "DEBUG", "--log-file", "06V21.txt", "--defaultsFile=liquibase.h2-mem.properties", "update", "--changelog-file", "postgres_lbpro_master_changelog.xml", "--labels", "setup"] | ["--log-level", "DEBUG", "--log-file", "06V21.txt", "--defaultsFile=liquibase.h2-mem.properties", "update", "--changelog-file", "postgres_lbpro_master_changelog.xml", "--labels", "setup"]
}

def "accepts -D subcommand arguments for changelog parameters"() {
Expand All @@ -76,4 +76,20 @@ class LiquibaseCommandLineTest extends Specification {
subcommands["update"].commandSpec.findOption("-D") != null
subcommands["snapshot"].commandSpec.findOption("-D") == null
}

@Unroll
def "cleanExceptionMessage"() {
expect:
new LiquibaseCommandLine().cleanExceptionMessage(input) == expected

where:
input | expected
null | null
"" | ""
"random string" | "random string"
"Unexpected error running Liquibase: message here" | "message here"
"java.lang.RuntimeException: message here" | "message here"
"java.lang.ParseError: message here" | "message here"
"java.io.RuntimeException: java.lang.RuntimeException: message here" | "message here"
}
}
6 changes: 5 additions & 1 deletion liquibase-core/src/main/java/liquibase/integration/commandline/Main.java
View file
Expand Up @@ -437,7 +437,11 @@ public Integer run() throws Exception {

e1.printStackTrace();
}
throw new LiquibaseException(String.format(coreBundle.getString("unexpected.error"), message), e);
if (runningFromNewCli) {
throw e;
} else {
throw new LiquibaseException(String.format(coreBundle.getString("unexpected.error"), message), e);
}
}

if (isHubEnabled(main.command) &&
Expand Down

0 comments on commit 16337ee

Please sign in to comment.