Skip to content

Commit 888ec29

Browse files
[8.1.0] Also apply --experimental_repository_downloader_retries to a SocketException (#24969)
Fix for #24530 --experimental_repository_downloader_retries will now retry on `SocketException` in addition to `ContentLengthMismatchException` Closes #24608. PiperOrigin-RevId: 704633572 Change-Id: Idd1fcbb768c9dabed596fe15d8ae9260ef3e895d Commit 459bb57 Co-authored-by: Pareesh Madan <[email protected]>
1 parent 916a36a commit 888ec29

File tree

2 files changed

+101
-2
lines changed

2 files changed

+101
-2
lines changed

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.google.devtools.build.lib.vfs.PathFragment;
3939
import java.io.IOException;
4040
import java.io.InterruptedIOException;
41+
import java.net.SocketException;
4142
import java.net.URI;
4243
import java.net.URISyntaxException;
4344
import java.net.URL;
@@ -357,19 +358,23 @@ private boolean shouldRetryDownload(IOException e, int attempt) {
357358
return false;
358359
}
359360

360-
if (e instanceof ContentLengthMismatchException) {
361+
if (isRetryableException(e)) {
361362
return true;
362363
}
363364

364365
for (var suppressed : e.getSuppressed()) {
365-
if (suppressed instanceof ContentLengthMismatchException) {
366+
if (isRetryableException(suppressed)) {
366367
return true;
367368
}
368369
}
369370

370371
return false;
371372
}
372373

374+
private boolean isRetryableException(Throwable e) {
375+
return e instanceof ContentLengthMismatchException || e instanceof SocketException;
376+
}
377+
373378
/**
374379
* Downloads the contents of one URL and reads it into a byte array.
375380
*

src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloaderTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import java.net.InetAddress;
4242
import java.net.ServerSocket;
4343
import java.net.Socket;
44+
import java.net.SocketException;
4445
import java.net.SocketTimeoutException;
4546
import java.net.URI;
4647
import java.net.URL;
@@ -773,6 +774,99 @@ public void download_contentLengthMismatchWithOtherErrors_retries() throws Excep
773774
assertThat(content).isEqualTo("content");
774775
}
775776

777+
@Test
778+
public void download_socketException_retries() throws Exception {
779+
Downloader downloader = mock(Downloader.class);
780+
HttpDownloader httpDownloader = mock(HttpDownloader.class);
781+
int retries = 5;
782+
DownloadManager downloadManager =
783+
new DownloadManager(repositoryCache, downloader, httpDownloader);
784+
downloadManager.setRetries(retries);
785+
AtomicInteger times = new AtomicInteger(0);
786+
byte[] data = "content".getBytes(UTF_8);
787+
doAnswer(
788+
(Answer<Void>)
789+
invocationOnMock -> {
790+
if (times.getAndIncrement() < 3) {
791+
throw new SocketException("Connection reset");
792+
}
793+
Path output = invocationOnMock.getArgument(5, Path.class);
794+
try (OutputStream outputStream = output.getOutputStream()) {
795+
ByteStreams.copy(new ByteArrayInputStream(data), outputStream);
796+
}
797+
798+
return null;
799+
})
800+
.when(downloader)
801+
.download(any(), any(), any(), any(), any(), any(), any(), any(), any());
802+
803+
Path result =
804+
download(
805+
downloadManager,
806+
ImmutableList.of(new URL("http://localhost")),
807+
ImmutableMap.of(),
808+
ImmutableMap.of(),
809+
Optional.empty(),
810+
"testCanonicalId",
811+
Optional.empty(),
812+
fs.getPath(workingDir.newFile().getAbsolutePath()),
813+
eventHandler,
814+
ImmutableMap.of(),
815+
"testRepo");
816+
817+
assertThat(times.get()).isEqualTo(4);
818+
String content = new String(ByteStreams.toByteArray(result.getInputStream()), UTF_8);
819+
assertThat(content).isEqualTo("content");
820+
}
821+
822+
@Test
823+
public void download_socketExceptionWithOtherErrors_retries() throws Exception {
824+
Downloader downloader = mock(Downloader.class);
825+
HttpDownloader httpDownloader = mock(HttpDownloader.class);
826+
int retries = 5;
827+
DownloadManager downloadManager =
828+
new DownloadManager(repositoryCache, downloader, httpDownloader);
829+
downloadManager.setRetries(retries);
830+
AtomicInteger times = new AtomicInteger(0);
831+
byte[] data = "content".getBytes(UTF_8);
832+
doAnswer(
833+
(Answer<Void>)
834+
invocationOnMock -> {
835+
if (times.getAndIncrement() < 3) {
836+
IOException e = new IOException();
837+
e.addSuppressed(new SocketException("Connection reset"));
838+
e.addSuppressed(new IOException());
839+
throw e;
840+
}
841+
Path output = invocationOnMock.getArgument(5, Path.class);
842+
try (OutputStream outputStream = output.getOutputStream()) {
843+
ByteStreams.copy(new ByteArrayInputStream(data), outputStream);
844+
}
845+
846+
return null;
847+
})
848+
.when(downloader)
849+
.download(any(), any(), any(), any(), any(), any(), any(), any(), any());
850+
851+
Path result =
852+
download(
853+
downloadManager,
854+
ImmutableList.of(new URL("http://localhost")),
855+
ImmutableMap.of(),
856+
ImmutableMap.of(),
857+
Optional.empty(),
858+
"testCanonicalId",
859+
Optional.empty(),
860+
fs.getPath(workingDir.newFile().getAbsolutePath()),
861+
eventHandler,
862+
ImmutableMap.of(),
863+
"testRepo");
864+
865+
assertThat(times.get()).isEqualTo(4);
866+
String content = new String(ByteStreams.toByteArray(result.getInputStream()), UTF_8);
867+
assertThat(content).isEqualTo("content");
868+
}
869+
776870
public Path download(
777871
DownloadManager downloadManager,
778872
List<URL> originalUrls,

0 commit comments

Comments
 (0)