Fix UX bug where event log was cut short during an error (#177)

halyard-cli/src/main/java/com/netflix/spinnaker/halyard/cli/command/v1/NestableCommand.java

@@ -20,6 +20,7 @@
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.ParameterDescription;
 import com.beust.jcommander.Parameters;
+import com.netflix.spinnaker.halyard.cli.services.v1.ExpectedDaemonFailureException;
 import com.netflix.spinnaker.halyard.cli.services.v1.ResponseUnwrapper;
 import com.netflix.spinnaker.halyard.cli.ui.v1.*;
 import com.netflix.spinnaker.halyard.core.DaemonResponse;
@@ -84,17 +85,12 @@ private void safeExecuteThis() {
         System.exit(1);
       }
 
-      try {
-        DaemonResponse d = (DaemonResponse) e.getBodyAs(DaemonResponse.class);
-        if (d.getProblemSet() != null) {
-          ResponseUnwrapper.get(d);
-          System.exit(1);
-        }
-      } catch (RuntimeException re) {
-      }
       AnsiUi.error(e.getMessage());
       AnsiUi.remediation("Try the command again with the --debug flag.");
       System.exit(1);
+    } catch (ExpectedDaemonFailureException e) {
+      AnsiUi.error("Operation failed. See above errors for details.");
+      System.exit(1);
     } catch (Exception e) {
       if (GlobalOptions.getGlobalOptions().isDebug()) {
         e.printStackTrace();

halyard-cli/src/main/java/com/netflix/spinnaker/halyard/cli/services/v1/DaemonService.java

@@ -125,7 +125,7 @@ DaemonTask<Halconfig, Void> setAccount(
       @Body Account account);
 
   @DELETE("/v1/config/deployments/{deploymentName}/providers/{providerName}/accounts/{accountName}/")
-  DaemonTask<Halconfig, Object> deleteAccount(
+  DaemonTask<Halconfig, Void> deleteAccount(
       @Path("deploymentName") String deploymentName,
       @Path("providerName") String providerName,
       @Path("accountName") String accountName,
@@ -143,7 +143,7 @@ DaemonTask<Halconfig, Void> setSecurity(
       @Body Security security);
 
   @GET("/v1/config/deployments/{deploymentName}/security/authn/{methodName}/")
-  DaemonTask<Halconfig, AuthnMethod> getAuthnMethod(
+  DaemonTask<Halconfig, Object> getAuthnMethod(
       @Path("deploymentName") String deploymentName,
       @Path("methodName") String methodName,
       @Query("validate") boolean validate);
@@ -203,7 +203,7 @@ DaemonTask<Halconfig, Void> setBaseImage(
       @Body BaseImage baseImage);
 
   @DELETE("/v1/config/deployments/{deploymentName}/providers/{providerName}/bakery/defaults/baseImage/{baseImageId}/")
-  DaemonTask<Halconfig, Object> deleteBaseImage(
+  DaemonTask<Halconfig, Void> deleteBaseImage(
       @Path("deploymentName") String deploymentName,
       @Path("providerName") String providerName,
       @Path("baseImageId") String baseImageId,

halyard-cli/src/main/java/com/netflix/spinnaker/halyard/cli/services/v1/ExpectedDaemonFailureException.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright 2017 Google, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License")
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package com.netflix.spinnaker.halyard.cli.services.v1;
+
+/**
+ * This is used for the CLI to indicate that the Daemon failed to process a request in an expected way.
+ *
+ * Examples include: An account couldn't be added because of a duplicate name.
+ *
+ * Non-examples include: The CLI encountered an NPE.
+ */
+public class ExpectedDaemonFailureException extends RuntimeException {
+  ExpectedDaemonFailureException() {
+    super();
+  }
+}

halyard-cli/src/main/java/com/netflix/spinnaker/halyard/cli/services/v1/ResponseUnwrapper.java

@@ -31,17 +31,13 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
-public class ResponseUnwrapper<T> {
+public class ResponseUnwrapper {
   private static final Long WAIT_MILLIS = 200L;
 
-  public static <T> T get(DaemonResponse<T> response) {
-    formatProblemSet(response.getProblemSet());
-    return response.getResponseBody();
-  }
-
   public static <C, T> T get(DaemonTask<C, T> task) {
     PrintCoordinates coords = new PrintCoordinates();
-    while (task.getState() != DaemonTask.State.SUCCESS) {
+
+    while (!task.getState().isTerminal()) {
       coords = formatStages(task.getStages(), coords);
 
       try {
@@ -56,12 +52,24 @@ public static <C, T> T get(DaemonTask<C, T> task) {
     AnsiSnippet clear = new AnsiSnippet("").setErase(AnsiErase.ERASE_START_LINE);
     AnsiPrinter.print(clear.toString());
 
-    return get(task.getResponse());
+    DaemonResponse<T> response = task.getResponse();
+    formatProblemSet(response.getProblemSet());
+    if (task.getState() == DaemonTask.State.FATAL) {
+      throw new ExpectedDaemonFailureException();
+    }
+
+    return response.getResponseBody();
   }
 
   private static PrintCoordinates formatStages(List<DaemonStage> stages, PrintCoordinates coords) {
     for (DaemonStage stage : stages.subList(coords.getLastStage(), stages.size())) {
-      coords = formatEvents(stage.getName(), stage.getEvents(), coords);
+      String stageName = stage.getName();
+      AnsiSnippet snippet = new AnsiSnippet("~ " + stageName)
+          .addStyle(AnsiStyle.BOLD)
+          .setErase(AnsiErase.ERASE_START_LINE);
+      AnsiPrinter.print(snippet.toString());
+
+      coords = formatEvents(stageName, stage.getEvents(), coords);
 
       if (stage.getState() == State.INACTIVE) {
         coords.setLastEvent(0);
@@ -86,7 +94,7 @@ private static PrintCoordinates formatEvents(String stageName, List<DaemonEvent>
     return coords;
   }
 
-  private static void formatProblemSet(ProblemSet problemSet) {
+  public static void formatProblemSet(ProblemSet problemSet) {
     if (problemSet == null) {
       return;
     }

halyard-core/src/main/java/com/netflix/spinnaker/halyard/core/problem/v1/ProblemSet.java

@@ -52,6 +52,10 @@ public void add(Problem problem) {
     problems.add(problem);
   }
 
+  public void addAll(ProblemSet problemSet) {
+    problems.addAll(problemSet.getProblems());
+  }
+
   public ProblemSet(List<Problem> problems) {
     this.problems = problems;
   }
@@ -76,7 +80,7 @@ private Problem.Severity maxSeverity() {
   }
 
   /**
-   * Problemhis is can be used to ignore errors that user deems frivolous.
+   * This is can be used to ignore errors that user deems frivolous.
    *
    * Example: A client's Jenkins instance isn't connecting to Halyard, but they are sure it will connect to Igor, so they
    * can force halyard to only generate an error if the severity exceeds "FAProblemAL" (which is impossible).

halyard-core/src/main/java/com/netflix/spinnaker/halyard/core/tasks/v1/DaemonTask.java

@@ -53,6 +53,10 @@ public enum State {
     NOT_STARTED,
     RUNNING,
     SUCCESS,
-    FATAL
+    FATAL;
+
+    public boolean isTerminal() {
+      return this == SUCCESS || this == FATAL;
+    }
   }
 }

halyard-core/src/main/java/com/netflix/spinnaker/halyard/core/tasks/v1/TaskRepository.java

@@ -2,6 +2,7 @@
 
 import com.netflix.spinnaker.halyard.core.DaemonResponse;
 import com.netflix.spinnaker.halyard.core.error.v1.HalException;
+import com.netflix.spinnaker.halyard.core.problem.v1.ProblemSet;
 import com.netflix.spinnaker.halyard.core.tasks.v1.DaemonTask.State;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
@@ -72,11 +73,18 @@ static public <C, T> DaemonTask<C, T> getTask(String uuid) {
 
     if (fatalError != null) {
       if (fatalError instanceof HalException) {
-        throw (HalException) fatalError;
+        HalException halException = (HalException) fatalError;
+        ProblemSet problemSet = halException.getProblems();
+        if (task.getResponse() != null) {
+          task.getResponse().getProblemSet().addAll(problemSet);
+        } else {
+          task.setResponse(new DaemonResponse<>(null, problemSet));
+        }
       } else {
-        throw new RuntimeException("Failed running task: " + fatalError.getMessage(), fatalError);
+        throw new RuntimeException("Unknown error encountered while running task: " + fatalError.getMessage(), fatalError);
       }
     }
+
     return task;
   }