Handle well understood HTTP exceptions in config server client

4 files changed, 39 insertions, 24 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
index 0e6000c651b..3d1c49fc166 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
@@ -106,7 +106,7 @@ public class ConfigServerApiImpl implements ConfigServerApi {
                 try {
                     return mapper.readValue(response.getEntity().getContent(), wantedReturnType);
                 } catch (IOException e) {
-                    throw new RuntimeException("Failed parse response from config server", e);
+                    throw new UncheckedIOException("Failed parse response from config server", e);
                 }
             } catch (HttpException e) {
                 if (!e.isRetryable()) throw e;
@@ -124,8 +124,8 @@ public class ConfigServerApiImpl implements ConfigServerApi {
             }
         }
 
-        throw new RuntimeException("All requests against the config servers ("
-                + configServers + ") failed, last as follows:", lastException);
+        throw HttpException.handleException(
+                "All requests against the config servers (" + configServers + ") failed, last as follows:", lastException);
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java
index 256fe38ec68..3825107bfa6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java
@@ -1,13 +1,19 @@
 // Copyright 2017 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.configserver;
 
+import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
+import org.apache.http.NoHttpResponseException;
+
 import javax.ws.rs.core.Response;
+import java.io.EOFException;
+import java.net.SocketException;
+import java.net.SocketTimeoutException;
 
 /**
  * @author hakonhall
  */
 @SuppressWarnings("serial")
-public class HttpException extends RuntimeException {
+public class HttpException extends ConvergenceException {
 
     private final boolean isRetryable;
 
@@ -21,7 +27,12 @@ public class HttpException extends RuntimeException {
         this.isRetryable = isRetryable;
     }
 
-    public boolean isRetryable() {
+    private HttpException(String message) {
+        super(message);
+        this.isRetryable = false;
+    }
+
+    boolean isRetryable() {
         return isRetryable;
     }
 
@@ -55,6 +66,22 @@ public class HttpException extends RuntimeException {
         throw new HttpException(status, message, true);
     }
 
+    /**
+     * Returns {@link HttpException} if the given Throwable is of a known and well understood error or
+     * a RuntimeException with the given exception as cause otherwise.
+     */
+    public static RuntimeException handleException(String prefix, Throwable t) {
+        for (; t != null; t = t.getCause()) {
+            if (t instanceof SocketException ||
+                t instanceof SocketTimeoutException ||
+                t instanceof NoHttpResponseException ||
+                t instanceof EOFException)
+                return new HttpException(prefix + t.getMessage());
+        }
+
+        return new RuntimeException(prefix, t);
+    }
+
     public static class NotFoundException extends HttpException {
         public NotFoundException(String message) {
             super(Response.Status.NOT_FOUND, message, false);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java
index efeb3039379..2fe8d4b4792 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImpl.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.configserver.state;
 
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
+import com.yahoo.vespa.hosted.node.admin.configserver.HttpException;
 import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse;
 
 /**
@@ -16,26 +17,12 @@ public class StateImpl implements State {
 
     @Override
     public HealthCode getHealth() {
-        HealthResponse response;
         try {
-            response = configServerApi.get("/state/v1/health", HealthResponse.class);
-        } catch (RuntimeException e) {
-            if (causedByConnectionRefused(e)) {
-                return HealthCode.DOWN;
-            }
-
-            throw e;
+            HealthResponse response = configServerApi.get("/state/v1/health", HealthResponse.class);
+            return HealthCode.fromString(response.status.code);
+        } catch (HttpException e) {
+            return HealthCode.DOWN;
         }
-        return HealthCode.fromString(response.status.code);
     }
 
-    private static boolean causedByConnectionRefused(Throwable throwable) {
-        for (Throwable cause = throwable; cause != null; cause = cause.getCause()) {
-            if (cause instanceof java.net.ConnectException) {
-                return true;
-            }
-        }
-
-        return false;
-    }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java
index 2604aa05367..14755ebf9cc 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/state/StateImplTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.configserver.state;
 
 import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi;
+import com.yahoo.vespa.hosted.node.admin.configserver.HttpException;
 import com.yahoo.vespa.hosted.node.admin.configserver.state.bindings.HealthResponse;
 import org.junit.Test;
 
@@ -28,7 +29,7 @@ public class StateImplTest {
 
     @Test
     public void connectException() {
-        RuntimeException exception = new RuntimeException(new ConnectException("connection refused"));
+        RuntimeException exception = HttpException.handleException("Error: ", new ConnectException("connection refused"));
         when(api.get(any(), any())).thenThrow(exception);
 
         HealthCode code = state.getHealth();