[FIXED JENKINS-26196] More robust handling of server errors in FilePa…

…th.installIfNecessaryFrom.
jenkinsci · Dec 29, 2014 · 1fcee97 · 1fcee97
1 parent babfb83
commit 1fcee97
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 6 deletions.
diff --git a/changelog.html b/changelog.html
@@ -63,6 +63,9 @@
   <li class=bug>
     Do not throw exception on <code>/signup</code> when not possible.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-11172">issue 11172</a>)
+  <li class=bug>
+    Tool installer which downloads and unpacks archives should not fail the build if the tool already exists and the server returns an error code.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-26196">issue 26196</a>)
   <li class=bug>
     Fingerprint compaction aggravated lazy-loading performance issues.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-19392">issue 19392</a>)

diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -756,11 +756,12 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
     public boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskListener listener, @Nonnull String message) throws IOException, InterruptedException {
         try {
             FilePath timestamp = this.child(".timestamp");
+            long lastModified = timestamp.lastModified();
             URLConnection con;
             try {
                 con = ProxyConfiguration.open(archive);
-                if (timestamp.exists()) {
-                    con.setIfModifiedSince(timestamp.lastModified());
+                if (lastModified != 0) {
+                    con.setIfModifiedSince(lastModified);
                 }
                 con.connect();
             } catch (IOException x) {
@@ -775,15 +776,21 @@ public boolean installIfNecessaryFrom(@Nonnull URL archive, @CheckForNull TaskLi
                 }
             }
 
-            if (con instanceof HttpURLConnection
-                    && ((HttpURLConnection)con).getResponseCode() == HttpURLConnection.HTTP_NOT_MODIFIED) {
-                return false;
+            if (lastModified != 0 && con instanceof HttpURLConnection) {
+                HttpURLConnection httpCon = (HttpURLConnection) con;
+                int responseCode = httpCon.getResponseCode();
+                if (responseCode == HttpURLConnection.HTTP_NOT_MODIFIED) {
+                    return false;
+                } else if (responseCode != HttpURLConnection.HTTP_OK) {
+                    listener.getLogger().println("Skipping installation of " + archive + " to " + remote + " due to server error: " + responseCode + " " + httpCon.getResponseMessage());
+                    return false;
+                }
             }
 
             long sourceTimestamp = con.getLastModified();
 
             if(this.exists()) {
-                if(timestamp.exists() && sourceTimestamp ==timestamp.lastModified())
+                if (lastModified != 0 && sourceTimestamp == lastModified)
                     return false;   // already up to date
                 this.deleteContents();
             } else {

diff --git a/core/src/test/java/hudson/FilePathTest.java b/core/src/test/java/hudson/FilePathTest.java
@@ -27,13 +27,15 @@
 import hudson.model.TaskListener;
 import hudson.remoting.VirtualChannel;
 import hudson.util.NullStream;
+import hudson.util.StreamTaskListener;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.ConnectException;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
@@ -545,6 +547,25 @@ private static void assertValidateAntFileMask(String expected, FilePath d, Strin
             assertTrue(d.installIfNecessaryFrom(url, null, null));
     }
 
+    @Issue("JENKINS-26196")
+    @Test public void installIfNecessarySkipsDownloadWhenErroneous() throws Exception {
+        File tmp = temp.getRoot();
+        final FilePath d = new FilePath(tmp);
+        d.child(".timestamp").touch(123000);
+        final HttpURLConnection con = mock(HttpURLConnection.class);
+        final URL url = someUrlToZipFile(con);
+        when(con.getResponseCode()).thenReturn(HttpURLConnection.HTTP_GATEWAY_TIMEOUT);
+        when(con.getResponseMessage()).thenReturn("Gateway Timeout");
+        when(con.getInputStream()).thenThrow(new ConnectException());
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        String message = "going ahead";
+        assertFalse(d.installIfNecessaryFrom(url, new StreamTaskListener(baos), message));
+        verify(con).setIfModifiedSince(123000);
+        String log = baos.toString();
+        assertFalse(log, log.contains(message));
+        assertTrue(log, log.contains("504 Gateway Timeout"));
+    }
+
     private URL someUrlToZipFile(final URLConnection con) throws IOException {
 
         final URLStreamHandler urlHandler = new URLStreamHandler() {