From f3f07d1d8bcb4a3a3ced31aeba37ff3a6a4daa2e Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Tue, 19 Jul 2022 11:05:13 -0500 Subject: [PATCH 1/2] Improve CLI error message by including "caused by" --- .../commandline/LiquibaseCommandLine.java | 44 ++++++++++++------- .../integration/commandline/Main.java | 6 ++- 2 files changed, 33 insertions(+), 17 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 2cfe6f1311d..90d77d6a3ea 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,15 @@ private int handleException(Throwable exception) { return 1; } + private String cleanExceptionMessage(String message) { + if (message == null) { + return null; + } + message = message.replaceFirst("^[\\w.]*exception[\\w.]*: ", ""); + message = message.replace("Unexpected error running Liquibase: ", ""); + return message; + } + public int execute(String[] args) { try { final String[] finalArgs = adjustLegacyArgs(args); 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) && From 4a45efe9850934bf995c3ae3c04f3410e2b2ffcf Mon Sep 17 00:00:00 2001 From: Nathan Voxland Date: Tue, 19 Jul 2022 11:37:23 -0500 Subject: [PATCH 2/2] Improve CLI error message by including "caused by" --- .../commandline/LiquibaseCommandLine.java | 11 ++++- .../commandline/LiquibaseLauncher.java | 19 +++++++- .../LiquibaseCommandLineTest.groovy | 46 +++++++++++++------ 3 files changed, 57 insertions(+), 19 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 90d77d6a3ea..5c2e85686c2 100644 --- a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java +++ b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseCommandLine.java @@ -289,11 +289,18 @@ private int handleException(Throwable exception) { return 1; } - private String cleanExceptionMessage(String message) { + protected String cleanExceptionMessage(String message) { if (message == null) { return null; } - message = message.replaceFirst("^[\\w.]*exception[\\w.]*: ", ""); + + 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; } 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 ac56d997914..66cd4499e78 100644 --- a/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java +++ b/liquibase-cli/src/main/java/liquibase/integration/commandline/LiquibaseLauncher.java @@ -24,6 +24,11 @@ public static void main(final String[] args) throws Exception { debug("Debug mode enabled because LIQUIBASE_LAUNCHER_DEBUG is set to " + debugSetting); } + String parentLoaderSetting = System.getenv("LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER"); + if (parentLoaderSetting == null) { + parentLoaderSetting = "system"; + } + debug("LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER is set to " + parentLoaderSetting); final String liquibaseHomeEnv = System.getenv("LIQUIBASE_HOME"); debug("LIQUIBASE_HOME: " + liquibaseHomeEnv); if (liquibaseHomeEnv == null || liquibaseHomeEnv.equals("")) { @@ -80,10 +85,20 @@ 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. - //That causes the parent classloader to load LiqiuabaseCommandLine which makes it not able to access files in the child classloader + //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.jar the same as the rest of the classes it needs to access. - final URLClassLoader classloader = new URLClassLoader(urls.toArray(new URL[0]), ClassLoader.getSystemClassLoader().getParent()); + parentLoader = ClassLoader.getSystemClassLoader().getParent(); + + } else if (parentLoaderSetting.equalsIgnoreCase("thread")) { + parentLoader = Thread.currentThread().getContextClassLoader(); + } else { + throw new RuntimeException("Unknown LIQUIBASE_LAUNCHER_PARENT_CLASSLOADER value: "+parentLoaderSetting); + } + + final URLClassLoader classloader = new URLClassLoader(urls.toArray(new URL[0]), parentLoader); Thread.currentThread().setContextClassLoader(classloader); final Class cli = classloader.loadClass(LiquibaseCommandLine.class.getName()); 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" + } }