Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve CLI error messages #3078

Merged
merged 4 commits into from Jul 28, 2022
Merged

Improve CLI error messages #3078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f3f07d1
Improve CLI error message by including "caused by"
nvoxland Jul 19, 2022
4a45efe
Improve CLI error message by including "caused by"
nvoxland Jul 19, 2022
03c44e7
Merge remote-tracking branch 'origin/master' into improve-cli-error-m…
nvoxland Jul 20, 2022
3de09e3
Merge branch 'master' into improve-cli-error-message
nvoxland Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 35 additions & 16 deletions liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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