Also apply --experimental_repository_downloader_retries to a SocketEx…

…ception Fix for #24530 --experimental_repository_downloader_retries will now retry on `SocketException` in addition to `ContentLengthMismatchException` Closes #24608. PiperOrigin-RevId: 704633572 Change-Id: Idd1fcbb768c9dabed596fe15d8ae9260ef3e895d
bazelbuild · Dec 10, 2024 · 459bb57 · 459bb57
1 parent 9ed06ad
commit 459bb57
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 2 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java
@@ -38,6 +38,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
 import java.io.InterruptedIOException;
+import java.net.SocketException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -357,19 +358,23 @@ private boolean shouldRetryDownload(IOException e, int attempt) {
       return false;
     }
 
-    if (e instanceof ContentLengthMismatchException) {
+    if (isRetryableException(e)) {
       return true;
     }
 
     for (var suppressed : e.getSuppressed()) {
-      if (suppressed instanceof ContentLengthMismatchException) {
+      if (isRetryableException(suppressed)) {
         return true;
       }
     }
 
     return false;
   }
 
+  private boolean isRetryableException(Throwable e) {
+    return e instanceof ContentLengthMismatchException || e instanceof SocketException;
+  }
+
   /**
    * Downloads the contents of one URL and reads it into a byte array.
    *

diff --git a/...st/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java b/...st/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java
@@ -41,6 +41,7 @@
 import java.net.InetAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
+import java.net.SocketException;
 import java.net.SocketTimeoutException;
 import java.net.URI;
 import java.net.URL;
@@ -773,6 +774,99 @@ public void download_contentLengthMismatchWithOtherErrors_retries() throws Excep
     assertThat(content).isEqualTo("content");
   }
 
+  @Test
+  public void download_socketException_retries() throws Exception {
+    Downloader downloader = mock(Downloader.class);
+    HttpDownloader httpDownloader = mock(HttpDownloader.class);
+    int retries = 5;
+    DownloadManager downloadManager =
+        new DownloadManager(repositoryCache, downloader, httpDownloader);
+    downloadManager.setRetries(retries);
+    AtomicInteger times = new AtomicInteger(0);
+    byte[] data = "content".getBytes(UTF_8);
+    doAnswer(
+            (Answer<Void>)
+                invocationOnMock -> {
+                  if (times.getAndIncrement() < 3) {
+                    throw new SocketException("Connection reset");
+                  }
+                  Path output = invocationOnMock.getArgument(5, Path.class);
+                  try (OutputStream outputStream = output.getOutputStream()) {
+                    ByteStreams.copy(new ByteArrayInputStream(data), outputStream);
+                  }
+
+                  return null;
+                })
+        .when(downloader)
+        .download(any(), any(), any(), any(), any(), any(), any(), any(), any());
+
+    Path result =
+        download(
+            downloadManager,
+            ImmutableList.of(new URL("http://localhost")),
+            ImmutableMap.of(),
+            ImmutableMap.of(),
+            Optional.empty(),
+            "testCanonicalId",
+            Optional.empty(),
+            fs.getPath(workingDir.newFile().getAbsolutePath()),
+            eventHandler,
+            ImmutableMap.of(),
+            "testRepo");
+
+    assertThat(times.get()).isEqualTo(4);
+    String content = new String(ByteStreams.toByteArray(result.getInputStream()), UTF_8);
+    assertThat(content).isEqualTo("content");
+  }
+
+  @Test
+  public void download_socketExceptionWithOtherErrors_retries() throws Exception {
+    Downloader downloader = mock(Downloader.class);
+    HttpDownloader httpDownloader = mock(HttpDownloader.class);
+    int retries = 5;
+    DownloadManager downloadManager =
+        new DownloadManager(repositoryCache, downloader, httpDownloader);
+    downloadManager.setRetries(retries);
+    AtomicInteger times = new AtomicInteger(0);
+    byte[] data = "content".getBytes(UTF_8);
+    doAnswer(
+            (Answer<Void>)
+                invocationOnMock -> {
+                  if (times.getAndIncrement() < 3) {
+                    IOException e = new IOException();
+                    e.addSuppressed(new SocketException("Connection reset"));
+                    e.addSuppressed(new IOException());
+                    throw e;
+                  }
+                  Path output = invocationOnMock.getArgument(5, Path.class);
+                  try (OutputStream outputStream = output.getOutputStream()) {
+                    ByteStreams.copy(new ByteArrayInputStream(data), outputStream);
+                  }
+
+                  return null;
+                })
+        .when(downloader)
+        .download(any(), any(), any(), any(), any(), any(), any(), any(), any());
+
+    Path result =
+        download(
+            downloadManager,
+            ImmutableList.of(new URL("http://localhost")),
+            ImmutableMap.of(),
+            ImmutableMap.of(),
+            Optional.empty(),
+            "testCanonicalId",
+            Optional.empty(),
+            fs.getPath(workingDir.newFile().getAbsolutePath()),
+            eventHandler,
+            ImmutableMap.of(),
+            "testRepo");
+
+    assertThat(times.get()).isEqualTo(4);
+    String content = new String(ByteStreams.toByteArray(result.getInputStream()), UTF_8);
+    assertThat(content).isEqualTo("content");
+  }
+
   public Path download(
       DownloadManager downloadManager,
       List<URL> originalUrls,