From 16337eee2f92e514ddc843064fc39bafcdd4e726 Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Thu, 28 Jul 2022 08:41:40 -0700 Subject: [PATCH] Improve CLI error messages (#3078) Improve CLI error message by including "caused by" --- .../commandline/LiquibaseCommandLine.java | 51 +++++++++++++------ .../commandline/LiquibaseLauncher.java | 4 +- .../LiquibaseCommandLineTest.groovy | 46 +++++++++++------ .../integration/commandline/Main.java | 6 ++- 4 files changed, 73 insertions(+), 34 deletions(-) diff --git a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java index fabe9375e3f..eade3af2de5 100644 --- a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java +++ b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java @@ -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; @@ -240,7 +243,7 @@ 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); @@ -248,12 +251,12 @@ private int handleException(Throwable exception) { } 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)) { @@ -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); diff --git a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java index 2783e7fd216..fba53a8ddff 100644 --- a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java +++ b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java @@ -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")) { diff --git a/liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy b/liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy index 37195364947..03e19ef2fe7 100644 --- a/liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy +++ b/liquibase-cli/src/test/groovy/liquibase/integration/commandline/LiquibaseCommandLineTest.groovy @@ -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 @@ -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"() { @@ -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" + } } diff --git a/liquibase-core/src/main/java/liquibase/integration/commandline/Main.java b/liquibase-core/src/main/java/liquibase/integration/commandline/Main.java index 054e8dd600c..429b2c0f3d9 100644 --- a/liquibase-core/src/main/java/liquibase/integration/commandline/Main.java +++ b/liquibase-core/src/main/java/liquibase/integration/commandline/Main.java @@ -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) &&