From 64afcd8e751ec2095c66c6b14f627c20b099b12d Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Tue, 23 Mar 2021 10:54:16 -0400 Subject: [PATCH] [Jib CLI] log exception with ConsoleLogger (#3162) * Log exception with ConsoleLogger * Update CHANGELOG --- jib-cli/CHANGELOG.md | 2 ++ .../com/google/cloud/tools/jib/cli/Build.java | 21 ++++++------- .../com/google/cloud/tools/jib/cli/Jar.java | 21 ++++++------- .../google/cloud/tools/jib/cli/JibCli.java | 21 +++++++++++++ .../cloud/tools/jib/cli/JibCliTest.java | 31 +++++++++++++++++++ 5 files changed, 72 insertions(+), 24 deletions(-) diff --git a/jib-cli/CHANGELOG.md b/jib-cli/CHANGELOG.md index aa9f4b7286..3eaa3f4c5a 100644 --- a/jib-cli/CHANGELOG.md +++ b/jib-cli/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file. ### Fixed +- Fixed an issue where critical error messages (for example, unauthorized access from a registry) were erased by progress reporting and not shown. ([#3148](https://github.com/GoogleContainerTools/jib/issues/3148)) + ## 0.4.0 ### Added diff --git a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Build.java b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Build.java index fa53d2c244..417778e478 100644 --- a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Build.java +++ b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Build.java @@ -98,15 +98,15 @@ public Integer call() { commonCliOptions.validate(); Path buildFile = getBuildFile(); SingleThreadedExecutor executor = new SingleThreadedExecutor(); + ConsoleLogger logger = + CliLogger.newLogger( + commonCliOptions.getVerbosity(), + commonCliOptions.getHttpTrace(), + commonCliOptions.getConsoleOutput(), + spec.commandLine().getOut(), + spec.commandLine().getErr(), + executor); try { - ConsoleLogger logger = - CliLogger.newLogger( - commonCliOptions.getVerbosity(), - commonCliOptions.getHttpTrace(), - commonCliOptions.getConsoleOutput(), - spec.commandLine().getOut(), - spec.commandLine().getErr(), - executor); JibCli.configureHttpLogging(commonCliOptions.getHttpTrace().toJulLevel()); if (!Files.isReadable(buildFile)) { @@ -133,10 +133,7 @@ public Integer call() { containerBuilder.containerize(containerizer); } catch (Exception ex) { - if (commonCliOptions.isStacktrace()) { - ex.printStackTrace(); - } - System.err.println(ex.getClass().getName() + ": " + ex.getMessage()); + JibCli.logTerminatingException(logger, ex, commonCliOptions.isStacktrace()); return 1; } finally { executor.shutDownAndAwaitTermination(Duration.ofSeconds(3)); diff --git a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Jar.java b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Jar.java index 1a344ab345..eb2f9d89f0 100644 --- a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Jar.java +++ b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/Jar.java @@ -165,15 +165,15 @@ public class Jar implements Callable { public Integer call() { commonCliOptions.validate(); SingleThreadedExecutor executor = new SingleThreadedExecutor(); + ConsoleLogger logger = + CliLogger.newLogger( + commonCliOptions.getVerbosity(), + commonCliOptions.getHttpTrace(), + commonCliOptions.getConsoleOutput(), + spec.commandLine().getOut(), + spec.commandLine().getErr(), + executor); try { - ConsoleLogger logger = - CliLogger.newLogger( - commonCliOptions.getVerbosity(), - commonCliOptions.getHttpTrace(), - commonCliOptions.getConsoleOutput(), - spec.commandLine().getOut(), - spec.commandLine().getErr(), - executor); JibCli.configureHttpLogging(commonCliOptions.getHttpTrace().toJulLevel()); if (!Files.exists(jarFile)) { @@ -204,10 +204,7 @@ public Integer call() { containerBuilder.containerize(containerizer); } catch (Exception ex) { - if (commonCliOptions.isStacktrace()) { - ex.printStackTrace(); - } - System.err.println(ex.getClass().getName() + ": " + ex.getMessage()); + JibCli.logTerminatingException(logger, ex, commonCliOptions.isStacktrace()); return 1; } finally { executor.shutDownAndAwaitTermination(Duration.ofSeconds(3)); diff --git a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/JibCli.java b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/JibCli.java index fb04566379..37ef16f8a6 100644 --- a/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/JibCli.java +++ b/jib-cli/src/main/java/com/google/cloud/tools/jib/cli/JibCli.java @@ -18,6 +18,10 @@ import com.google.api.client.http.HttpTransport; import com.google.api.client.http.apache.v2.ApacheHttpTransport; +import com.google.cloud.tools.jib.api.LogEvent; +import com.google.cloud.tools.jib.plugins.common.logging.ConsoleLogger; +import java.io.PrintWriter; +import java.io.StringWriter; import java.util.logging.ConsoleHandler; import java.util.logging.Level; import java.util.logging.Logger; @@ -46,6 +50,23 @@ static Logger configureHttpLogging(Level level) { return logger; } + static void logTerminatingException( + ConsoleLogger consoleLogger, Exception exception, boolean logStackTrace) { + if (logStackTrace) { + StringWriter writer = new StringWriter(); + exception.printStackTrace(new PrintWriter(writer)); + consoleLogger.log(LogEvent.Level.ERROR, writer.toString()); + } + + consoleLogger.log( + LogEvent.Level.ERROR, + "\u001B[31;1m" + + exception.getClass().getName() + + ": " + + exception.getMessage() + + "\u001B[0m"); + } + /** * The magic starts here. * diff --git a/jib-cli/src/test/java/com/google/cloud/tools/jib/cli/JibCliTest.java b/jib-cli/src/test/java/com/google/cloud/tools/jib/cli/JibCliTest.java index d2e6b0da6a..131e28bbb4 100644 --- a/jib-cli/src/test/java/com/google/cloud/tools/jib/cli/JibCliTest.java +++ b/jib-cli/src/test/java/com/google/cloud/tools/jib/cli/JibCliTest.java @@ -17,7 +17,15 @@ package com.google.cloud.tools.jib.cli; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.google.cloud.tools.jib.api.LogEvent; +import com.google.cloud.tools.jib.plugins.common.logging.ConsoleLogger; +import java.io.IOException; import java.util.logging.ConsoleHandler; import java.util.logging.Handler; import java.util.logging.Level; @@ -37,4 +45,27 @@ public void testConfigureHttpLogging() { assertThat(handler).isInstanceOf(ConsoleHandler.class); assertThat(handler.getLevel()).isEqualTo(Level.ALL); } + + @Test + public void testLogTerminatingException() { + ConsoleLogger logger = mock(ConsoleLogger.class); + JibCli.logTerminatingException(logger, new IOException("test error message"), false); + + verify(logger) + .log(LogEvent.Level.ERROR, "\u001B[31;1mjava.io.IOException: test error message\u001B[0m"); + verifyNoMoreInteractions(logger); + } + + @Test + public void testLogTerminatingException_stackTrace() { + ConsoleLogger logger = mock(ConsoleLogger.class); + JibCli.logTerminatingException(logger, new IOException("test error message"), true); + + String stackTraceLine = + "at com.google.cloud.tools.jib.cli.JibCliTest.testLogTerminatingException_stackTrace"; + verify(logger).log(eq(LogEvent.Level.ERROR), contains(stackTraceLine)); + verify(logger) + .log(LogEvent.Level.ERROR, "\u001B[31;1mjava.io.IOException: test error message\u001B[0m"); + verifyNoMoreInteractions(logger); + } }