commit aa4252417f54e28db3a7fa1f98d9a06d8ce49465
author David Pursehouse <dpursehouse@collab.net> Wed May 09 15:59:32 2018 +0900
committer David Pursehouse <dpursehouse@collab.net> Wed May 09 15:59:32 2018 +0900
tree 3ac4f89de4f4173848e40dfa248b3481aa751838
parent 1bcd25599cd37a50c15d8ac9fe98785ce96337a0
parent 1bed9a12bcade66280db6f70bc8d864de6bf37e9

Merge branch 'stable-2.15'

* stable-2.15:
  Log a warning when a hook exits with error status
  Include hook name in error message when logging output failed
  Log a warning when a hook times out
  Use Logger's built-in string formatting

Change-Id: Iac58aca64c9699f28ea105850e9fdefbea1e3e64

src/main/java/com/googlesource/gerrit/plugins/hooks/HookExecutor.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookExecutor.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookExecutor.java
index 5409227..4a5f6bb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookExecutor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookExecutor.java
@@ -22,7 +22,7 @@
       new UncaughtExceptionHandler() {
         @Override
         public void uncaughtException(Thread t, Throwable e) {
-          log.error("HookExecutor thread " + t.getName() + " threw exception", e);
+          log.error("HookExecutor thread {} threw exception", t.getName(), e);
         }
       };
 

src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
index 20bb088..7a7189a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/HookTask.java
@@ -113,22 +113,28 @@
     } catch (InterruptedException iex) {
       // InterruptedException - timeout or cancel
       args.metrics.timeout(name);
+      log.warn("hook[{}] timed out: {}", name, iex.getMessage());
     } catch (Throwable err) {
       args.metrics.error(name);
-      log.error("Error running hook " + hook.toAbsolutePath(), err);
+      log.error("Error running hook {}", hook.toAbsolutePath(), err);
     }
 
-    if (result != null && log.isDebugEnabled()) {
-      log.debug(String.format("hook[%s] exitValue: %d", name, result.getExitValue()));
+    if (result != null) {
+      int exitValue = result.getExitValue();
+      if (exitValue != 0) {
+        log.warn("hook[{}] exited with error status: {}", name, exitValue);
+      }
 
-      BufferedReader br = new BufferedReader(new StringReader(result.getOutput()));
-      try {
-        String line;
-        while ((line = br.readLine()) != null) {
-          log.debug(String.format("hook[%s] output: %s", name, line));
+      if (log.isDebugEnabled()) {
+        BufferedReader br = new BufferedReader(new StringReader(result.getOutput()));
+        try {
+          String line;
+          while ((line = br.readLine()) != null) {
+            log.debug("hook[{}] output: {}", name, line);
+          }
+        } catch (IOException iox) {
+          log.error("Error writing hook [{}] output", name, iox);
         }
-      } catch (IOException iox) {
-        log.error("Error writing hook output", iox);
       }
     }