[Jib CLI] log exception with ConsoleLogger (#3162)

* Log exception with ConsoleLogger * Update CHANGELOG
GoogleContainerTools · Mar 23, 2021 · 64afcd8 · 64afcd8
1 parent 3cbf30f
commit 64afcd8
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 24 deletions.
diff --git 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
@@ -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
@@ -165,15 +165,15 @@ public class Jar implements Callable<Integer> {
   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
@@ -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
@@ -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);
+  }
 }