Extract util for formatting exception message

6 files changed, 125 insertions, 47 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java
index f61764463ce..71a4c7c109b 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcess.java
@@ -15,13 +15,6 @@ public interface ChildProcess extends AutoCloseable {
     String getUtf8Output();
 
     /**
-     * Returns an UnexpectedOutputException that includes a snippet of the output in the message.
-     *
-     * @param problem Problem description, e.g. "Output is not of the form ^NAME=VALUE$"
-     */
-    UnexpectedOutputException newUnexpectedOutputException(String problem);
-
-    /**
      * Only call this if process was spawned under the assumption the program had no side
      * effects (see Command::spawnProgramWithoutSideEffects).  If it is determined later
      * that the program did in fact have side effects (modified system), this method can
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java
index 843ad3c5e69..98fa18db52d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ChildProcessImpl.java
@@ -13,11 +13,6 @@ import java.util.logging.Logger;
  * @author hakonhall
  */
 public class ChildProcessImpl implements ChildProcess {
-    public static final int MAX_OUTPUT_PREFIX = 200;
-    public static final int MAX_OUTPUT_SUFFIX = 200;
-    // Omitting a number of chars less than 10 or less than 10% would be ridiculous.
-    public static final int MAX_OUTPUT_SLACK = Math.max(10, (10 * (MAX_OUTPUT_PREFIX + MAX_OUTPUT_SUFFIX)) / 100);
-
     private final TaskContext taskContext;
     private final ProcessApi process;
     private final String commandLine;
@@ -67,7 +62,9 @@ public class ChildProcessImpl implements ChildProcess {
         waitForTermination();
         int exitCode = process.exitCode();
         if (exitCode != 0) {
-            String message = debugDescription("terminated with non-zero exit code " + exitCode);
+            String message = ErrorMessageFormatter.createSnippetForTerminatedProcess(
+                    "terminated with non-zero exit code " + exitCode,
+                    this);
             throw new CommandException(message);
         }
 
@@ -75,36 +72,6 @@ public class ChildProcessImpl implements ChildProcess {
     }
 
     @Override
-    public UnexpectedOutputException newUnexpectedOutputException(String problem) {
-        String message = debugDescription("output was not of the expected format: " + problem);
-        throw new UnexpectedOutputException(message);
-    }
-
-    private String debugDescription(String problem) {
-        StringBuilder stringBuilder = new StringBuilder()
-                .append("Command '")
-                .append(commandLine())
-                .append("' ")
-                .append(problem)
-                .append(": stdout/stderr: '");
-
-        String possiblyHugeOutput = getUtf8Output();
-        if (possiblyHugeOutput.length() <= MAX_OUTPUT_PREFIX + MAX_OUTPUT_SUFFIX + MAX_OUTPUT_SLACK) {
-            stringBuilder.append(possiblyHugeOutput);
-        } else {
-            stringBuilder.append(possiblyHugeOutput.substring(0, MAX_OUTPUT_PREFIX))
-                    .append("... [")
-                    .append(possiblyHugeOutput.length() - MAX_OUTPUT_PREFIX - MAX_OUTPUT_SUFFIX)
-                    .append(" chars omitted] ...")
-                    .append(possiblyHugeOutput.substring(possiblyHugeOutput.length() - MAX_OUTPUT_SUFFIX));
-        }
-
-        stringBuilder.append("'");
-
-        return stringBuilder.toString();
-    }
-
-    @Override
     public void logAsModifyingSystemAfterAll(Logger logger) {
         taskContext.logSystemModification(logger, "Executed command: " + commandLine);
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatter.java
new file mode 100644
index 00000000000..4a7eac7d4ea
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatter.java
@@ -0,0 +1,54 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.node.admin.task.util.process;
+
+/**
+ * @author hakonhall
+ */
+class ErrorMessageFormatter {
+    static final int MAX_OUTPUT_PREFIX = 200;
+    static final int MAX_OUTPUT_SUFFIX = 200;
+    // Omitting a number of chars less than 10 or less than 10% would be ridiculous.
+    static final int MAX_OUTPUT_SLACK = Math.max(10, (10 * (MAX_OUTPUT_PREFIX + MAX_OUTPUT_SUFFIX)) / 100);
+
+    /**
+     * Creates an error message suitable for an exception or logging, that includes the
+     * command line and output of a terminated process. The output is truncated if too large.
+     * Format on resulting message:
+     *   Command 'COMMANDLINE' PROBLEM: stdout/stderr: 'OUTPUT'
+     *
+     * @param problem           E.g. "terminated with exit code 1"
+     * @param childProcess      A terminated child process
+     */
+    static String createSnippetForTerminatedProcess(String problem, ChildProcess childProcess) {
+        return createSnippetForTerminatedProcessWith(
+                problem, childProcess, MAX_OUTPUT_PREFIX, MAX_OUTPUT_SUFFIX, MAX_OUTPUT_SLACK);
+    }
+
+    static String createSnippetForTerminatedProcessWith(String problem,
+                                                        ChildProcess childProcess,
+                                                        int maxOutputPrefix,
+                                                        int maxOutputSuffix,
+                                                        int maxOutputSlack) {
+        StringBuilder stringBuilder = new StringBuilder()
+                .append("Command '")
+                .append(childProcess.commandLine())
+                .append("' ")
+                .append(problem)
+                .append(": stdout/stderr: '");
+
+        String possiblyHugeOutput = childProcess.getUtf8Output();
+        if (possiblyHugeOutput.length() <= maxOutputPrefix + maxOutputSuffix + maxOutputSlack) {
+            stringBuilder.append(possiblyHugeOutput);
+        } else {
+            stringBuilder.append(possiblyHugeOutput.substring(0, maxOutputPrefix))
+                    .append("... [")
+                    .append(possiblyHugeOutput.length() - maxOutputPrefix - maxOutputSuffix)
+                    .append(" chars omitted] ...")
+                    .append(possiblyHugeOutput.substring(possiblyHugeOutput.length() - maxOutputSuffix));
+        }
+
+        stringBuilder.append("'");
+
+        return stringBuilder.toString();
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java
index 64ff044753a..24197a363ba 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/process/UnexpectedOutputException.java
@@ -6,7 +6,11 @@ package com.yahoo.vespa.hosted.node.admin.task.util.process;
  */
 @SuppressWarnings("serial")
 public class UnexpectedOutputException extends RuntimeException {
-    UnexpectedOutputException(String message) {
-        super(message);
+    /**
+     * @param problem Problem description, e.g. "Output is not of the form ^NAME=VALUE$"
+     */
+    public UnexpectedOutputException(String problem, ChildProcess childProcess) {
+        super(ErrorMessageFormatter.createSnippetForTerminatedProcess(
+                "output was not of the expected format: " + problem, childProcess));
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java
index bebf4ea85a8..c294d9ee806 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/systemd/SystemCtl.java
@@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.task.util.systemd;
 import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.ChildProcess;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.Command;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.UnexpectedOutputException;
 
 import java.util.Objects;
 import java.util.function.Function;
@@ -140,8 +141,8 @@ public class SystemCtl {
             String output = showProcess.getUtf8Output();
             Matcher matcher = propertyPattern.matcher(output);
             if (!matcher.find()) {
-                throw showProcess.newUnexpectedOutputException(
-                        "Output does not match '" + propertyPattern + "'");
+                throw new UnexpectedOutputException(
+                        "Output does not match '" + propertyPattern + "'", showProcess);
             } else if (matcher.groupCount() != 1) {
                 throw new IllegalArgumentException("Property pattern must have exactly 1 group");
             }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatterTest.java
new file mode 100644
index 00000000000..85bdd6f6e3c
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/process/ErrorMessageFormatterTest.java
@@ -0,0 +1,59 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.node.admin.task.util.process;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class ErrorMessageFormatterTest {
+    private String createFullSnippet(String problem, String commandLine, String output) {
+        return createSnippet(problem, commandLine, output, 1000, 1000, 1000);
+    }
+
+    private String createSnippet(String problem,
+                                 String commandLine,
+                                 String output,
+                                 int maxOutputPrefix,
+                                 int maxOutputSuffix,
+                                 int maxOutputSlack) {
+        ChildProcess childProcess = mock(ChildProcess.class);
+        when(childProcess.commandLine()).thenReturn(commandLine);
+        when(childProcess.getUtf8Output()).thenReturn(output);
+        return ErrorMessageFormatter.createSnippetForTerminatedProcessWith(
+                problem, childProcess, maxOutputPrefix, maxOutputSuffix, maxOutputSlack);
+    }
+
+    @Test
+    public void verifyWithoutTrimming() {
+        assertEquals("Command 'command line' problem: stdout/stderr: 'output'",
+                createFullSnippet("problem", "command line", "output"));
+    }
+
+    @Test
+    public void verifyWithTrimming() {
+        assertEquals(
+                "Command 'command line' problem: stdout/stderr: 'Thi... [15 chars omitted] ...sage'",
+                createSnippet(
+                        "problem",
+                        "command line",
+                        "This is a long message",
+                        3,
+                        4,
+                        14));
+    }
+
+    @Test
+    public void verifySlack() {
+        assertEquals(
+                "Command 'command line' problem: stdout/stderr: 'This is a long message'",
+                createSnippet(
+                        "problem",
+                        "command line",
+                        "This is a long message",
+                        3,
+                        4,
+                        16));
+    }
+}
\ No newline at end of file