Merge pull request #225 from basil/exceptions

Clean up exception handling
jenkinsci · Jun 6, 2020 · c189b56 · c189b56
2 parents 1d02b29 + 9f5c1cc
commit c189b56
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 64 deletions.
diff --git a/client/src/main/java/hudson/plugins/swarm/Client.java b/client/src/main/java/hudson/plugins/swarm/Client.java
@@ -15,6 +15,7 @@
 import java.lang.management.ManagementFactory;
 import java.net.InetAddress;
 import java.net.URL;
+import java.net.UnknownHostException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
@@ -35,8 +36,7 @@ public static void main(String... args) throws InterruptedException, IOException
         try {
             parser.parseArgument(args);
         } catch (CmdLineException e) {
-            logger.log(Level.SEVERE, "CmdLineException occurred during parseArgument", e);
-            parser.printUsage(System.out);
+            System.err.println(e.getMessage());
             System.exit(1);
         }
 
@@ -117,7 +117,7 @@ public static void main(String... args) throws InterruptedException, IOException
         if (options.name == null) {
             try {
                 client.options.name = InetAddress.getLocalHost().getCanonicalHostName();
-            } catch (IOException e) {
+            } catch (UnknownHostException e) {
                 logger.severe(
                         "Failed to look up the canonical hostname of this agent. Check the system"
                                 + " DNS settings.");
@@ -142,14 +142,13 @@ public Client(Options options) {
      * <p>This method never returns.
      */
     public void run(SwarmClient swarmClient, String... args) throws InterruptedException {
-        logger.info("Discovering Jenkins master");
+        logger.info("Connecting to Jenkins master");
+        URL masterUrl = swarmClient.getMasterUrl();
 
         // wait until we get the ACK back
         int retry = 0;
         while (true) {
             try {
-                URL masterUrl = swarmClient.getMasterUrl();
-
                 logger.info("Attempting to connect to " + masterUrl);
 
                 /*
@@ -179,14 +178,9 @@ public void run(SwarmClient swarmClient, String... args) throws InterruptedExcep
                     logger.warning("Connection closed, exiting...");
                     swarmClient.exitWithStatus(0);
                 }
-            } catch (IOException e) {
-                logger.log(Level.SEVERE, "IOException occurred", e);
+            } catch (IOException | RetryException e) {
+                logger.log(Level.SEVERE, "An error occurred", e);
                 e.printStackTrace();
-            } catch (RetryException e) {
-                logger.log(Level.SEVERE, "RetryException occurred", e);
-                if (e.getCause() != null) {
-                    e.getCause().printStackTrace();
-                }
             }
 
             int waitTime = options.retryBackOffStrategy.waitForRetry(retry++, options.retryInterval, options.maxRetryInterval);

diff --git a/client/src/main/java/hudson/plugins/swarm/LabelFileWatcher.java b/client/src/main/java/hudson/plugins/swarm/LabelFileWatcher.java
@@ -13,7 +13,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
@@ -57,7 +56,7 @@ public LabelFileWatcher(URL masterUrl, Options options, String name, String... a
     @SuppressFBWarnings(
             value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE",
             justification = "False positive for try-with-resources in Java 11")
-    private void softLabelUpdate(String sNewLabels)  throws SoftLabelUpdateException, MalformedURLException {
+    private void softLabelUpdate(String sNewLabels) throws SoftLabelUpdateException {
         // 1. get labels from master
         // 2. issue remove command for all old labels
         // 3. issue update commands for new labels

diff --git a/client/src/main/java/hudson/plugins/swarm/Options.java b/client/src/main/java/hudson/plugins/swarm/Options.java
@@ -123,7 +123,11 @@ public class Options {
     @Option(name = "-password", usage = "The Jenkins user API token or password.")
     public String password;
 
-    @Option(name = "-help", aliases = "--help", usage = "Show the help screen")
+    @Option(
+            name = "-help",
+            aliases = {"--help", "-h"},
+            help = true,
+            usage = "Show the help screen")
     public boolean help;
 
     @Option(

diff --git a/client/src/main/java/hudson/plugins/swarm/SwarmClient.java b/client/src/main/java/hudson/plugins/swarm/SwarmClient.java
@@ -33,6 +33,7 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.io.UnsupportedEncodingException;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
@@ -50,7 +51,6 @@
 import java.security.SecureRandom;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
-import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -116,9 +116,8 @@ public URL getMasterUrl() {
         try {
             return new URL(options.master);
         } catch (MalformedURLException e) {
-            String msg = MessageFormat.format("The master URL \"{0}\" is invalid", options.master);
-            logger.log(Level.SEVERE, msg, e);
-            throw new RuntimeException(msg, e);
+            throw new UncheckedIOException(
+                    String.format("The master URL %s is invalid", options.master), e);
         }
     }
 
@@ -127,21 +126,18 @@ public URL getMasterUrl() {
      *
      * <p>Interrupt the thread to abort it and return.
      */
-    protected void connect(URL masterUrl) throws InterruptedException {
+    protected void connect(URL masterUrl) throws InterruptedException, IOException, RetryException {
         logger.fine("connect() invoked");
 
         Launcher launcher = new Launcher();
+
         // prevent infinite retry in hudson.remoting.Launcher.parseJnlpArguments()
         launcher.noReconnect = true;
 
-        List<String> jnlpArgs = Collections.emptyList();
-
         try {
             launcher.agentJnlpURL = new URL(masterUrl + "computer/" + name + "/slave-agent.jnlp");
         } catch (MalformedURLException e) {
-            e.printStackTrace();
-            logger.log(Level.SEVERE, "Failed to establish JNLP connection to " + masterUrl, e);
-            Thread.sleep(10 * 1000);
+            throw new RetryException("Failed to establish JNLP connection to " + masterUrl, e);
         }
 
         if (options.username != null && options.password != null) {
@@ -157,12 +153,11 @@ protected void connect(URL masterUrl) throws InterruptedException {
             }
         }
 
+        List<String> jnlpArgs;
         try {
             jnlpArgs = launcher.parseJnlpArguments();
         } catch (Exception e) {
-            e.printStackTrace();
-            logger.log(Level.SEVERE, "Failed to establish JNLP connection to " + masterUrl, e);
-            Thread.sleep(10 * 1000);
+            throw new RetryException("Failed to establish JNLP connection to " + masterUrl, e);
         }
 
         List<String> args = new ArrayList<>();
@@ -189,7 +184,7 @@ protected void connect(URL masterUrl) throws InterruptedException {
         }
 
         if (!options.disableWorkDir) {
-            final String workDirPath =
+            String workDirPath =
                     options.workDir != null
                             ? options.workDir.getPath()
                             : options.remoteFsRoot.getPath();
@@ -221,9 +216,7 @@ protected void connect(URL masterUrl) throws InterruptedException {
         try {
             Main.main(args.toArray(new String[0]));
         } catch (Exception e) {
-            e.printStackTrace();
-            logger.log(Level.SEVERE, "Failed to establish JNLP connection to " + masterUrl, e);
-            Thread.sleep(10 * 1000);
+            throw new RetryException("Failed to establish JNLP connection to " + masterUrl, e);
         }
     }
 
@@ -399,13 +392,12 @@ protected void createSwarmAgent(URL masterUrl) throws IOException, RetryExceptio
 
         try (CloseableHttpResponse response = client.execute(post, context)) {
             if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-                String msg =
+                throw new RetryException(
                         String.format(
                                 "Failed to create a Swarm agent on Jenkins. Response code: %s%n%s",
                                 response.getStatusLine().getStatusCode(),
-                                EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8));
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                                EntityUtils.toString(
+                                        response.getEntity(), StandardCharsets.UTF_8)));
             }
 
             try (InputStream stream = response.getEntity().getContent()) {
@@ -469,13 +461,12 @@ protected static synchronized void postLabelRemove(
 
         try (CloseableHttpResponse response = client.execute(post, context)) {
             if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-                String msg =
+                throw new RetryException(
                         String.format(
                                 "Failed to remove agent labels. Response code: %s%n%s",
                                 response.getStatusLine().getStatusCode(),
-                                EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8));
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                                EntityUtils.toString(
+                                        response.getEntity(), StandardCharsets.UTF_8)));
             }
         }
     }
@@ -506,13 +497,12 @@ protected static synchronized void postLabelAppend(
 
         try (CloseableHttpResponse response = client.execute(post, context)) {
             if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-                String msg =
+                throw new RetryException(
                         String.format(
                                 "Failed to update agent labels. Response code: %s%n%s",
                                 response.getStatusLine().getStatusCode(),
-                                EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8));
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                                EntityUtils.toString(
+                                        response.getEntity(), StandardCharsets.UTF_8)));
             }
         }
     }
@@ -538,41 +528,31 @@ private static synchronized String param(String name, String value) throws Unsup
             justification = "False positive for try-with-resources in Java 11")
     protected VersionNumber getJenkinsVersion(
             CloseableHttpClient client, HttpClientContext context, URL masterUrl)
-            throws RetryException {
+            throws IOException, RetryException {
         logger.fine("getJenkinsVersion() invoked");
 
         HttpGet httpGet = new HttpGet(masterUrl + "api");
         try (CloseableHttpResponse response = client.execute(httpGet, context)) {
             if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-                String msg =
+                throw new RetryException(
                         String.format(
-                                "Could not get Jenkins version. Response code: Response code:"
-                                        + " %s%n%s",
+                                "Could not get Jenkins version. Response code: %s%n%s",
                                 response.getStatusLine().getStatusCode(),
-                                EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8));
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                                EntityUtils.toString(
+                                        response.getEntity(), StandardCharsets.UTF_8)));
             }
 
             Header[] headers = response.getHeaders("X-Jenkins");
             if (headers.length != 1) {
-                String msg = "This URL doesn't look like Jenkins.";
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                throw new RetryException("This URL doesn't look like Jenkins.");
             }
 
             String versionStr = headers[0].getValue();
             try {
                 return new VersionNumber(versionStr);
             } catch (RuntimeException e) {
-                String msg = "Unexpected Jenkins version: " + versionStr;
-                logger.log(Level.SEVERE, msg);
-                throw new RetryException(msg);
+                throw new RetryException("Unexpected Jenkins version: " + versionStr, e);
             }
-        } catch (IOException e) {
-            String msg = "Failed to connect to " + masterUrl;
-            logger.log(Level.SEVERE, msg, e);
-            throw new RetryException(msg);
         }
     }
 

diff --git a/plugin/src/test/java/hudson/plugins/swarm/SwarmClientIntegrationTest.java b/plugin/src/test/java/hudson/plugins/swarm/SwarmClientIntegrationTest.java
@@ -15,6 +15,8 @@
 import hudson.tasks.Shell;
 import hudson.util.VersionNumber;
 
+import jenkins.model.Jenkins;
+
 import org.apache.commons.lang.RandomStringUtils;
 import org.apache.commons.lang.math.NumberUtils;
 import org.junit.After;
@@ -234,8 +236,7 @@ public void pidFileForStaleProcessIsIgnored() throws Exception {
     @Test
     @Issue("JENKINS-61969")
     public void webSocket() throws Exception {
-        Assume.assumeTrue(
-                j.jenkins.getVersion().isNewerThanOrEqualTo(new VersionNumber("2.222.4")));
+        Assume.assumeTrue(Jenkins.getVersion().isNewerThanOrEqualTo(new VersionNumber("2.222.4")));
         Node node = swarmClientRule.createSwarmClient("-webSocket");
 
         FreeStyleProject project = j.createFreeStyleProject();