Prevent action-cache duplicate suppression

Exceptions that are reused between download futures must not be added to the suppression of themselves. Fixes #9362 Closes #9376. PiperOrigin-RevId: 270658205
bazelbuild · Oct 14, 2019 · 9cb225a · 9cb225a
1 parent 9c257df
commit 9cb225a
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 2 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -354,13 +354,13 @@ public void download(
       } catch (IOException e) {
         if (downloadException == null) {
           downloadException = e;
-        } else {
+        } else if (e != downloadException) {
           downloadException.addSuppressed(e);
         }
       } catch (InterruptedException e) {
         if (interruptedException == null) {
           interruptedException = e;
-        } else {
+        } else if (e != interruptedException) {
           interruptedException.addSuppressed(e);
         }
       }

diff --git a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
@@ -716,6 +716,64 @@ public void downloadWithMultipleErrorsAddsThemAsSuppressed() throws Exception {
     assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("file2 failed");
   }
 
+  @Test
+  public void downloadWithDuplicateIOErrorsDoesNotSuppress() throws Exception {
+    Path stdout = fs.getPath("/execroot/stdout");
+    Path stderr = fs.getPath("/execroot/stderr");
+
+    DefaultRemoteActionCache cache = newTestCache();
+    Digest digest1 = cache.addContents("file1");
+    IOException reusedException = new IOException("reused io exception");
+    Digest digest2 = cache.addException("file2", reusedException);
+    Digest digest3 = cache.addException("file3", reusedException);
+
+    ActionResult result =
+        ActionResult.newBuilder()
+            .setExitCode(0)
+            .addOutputFiles(OutputFile.newBuilder().setPath("file1").setDigest(digest1))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file2").setDigest(digest2))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file3").setDigest(digest3))
+            .build();
+    IOException e =
+        assertThrows(
+            IOException.class,
+            () ->
+                cache.download(
+                    result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker));
+
+    assertThat(e.getSuppressed()).isEmpty();
+    assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("reused io exception");
+  }
+
+  @Test
+  public void downloadWithDuplicateInterruptionsDoesNotSuppress() throws Exception {
+    Path stdout = fs.getPath("/execroot/stdout");
+    Path stderr = fs.getPath("/execroot/stderr");
+
+    DefaultRemoteActionCache cache = newTestCache();
+    Digest digest1 = cache.addContents("file1");
+    InterruptedException reusedInterruption = new InterruptedException("reused interruption");
+    Digest digest2 = cache.addException("file2", reusedInterruption);
+    Digest digest3 = cache.addException("file3", reusedInterruption);
+
+    ActionResult result =
+        ActionResult.newBuilder()
+            .setExitCode(0)
+            .addOutputFiles(OutputFile.newBuilder().setPath("file1").setDigest(digest1))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file2").setDigest(digest2))
+            .addOutputFiles(OutputFile.newBuilder().setPath("file3").setDigest(digest3))
+            .build();
+    InterruptedException e =
+        assertThrows(
+            InterruptedException.class,
+            () ->
+                cache.download(
+                    result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker));
+
+    assertThat(e.getSuppressed()).isEmpty();
+    assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("reused interruption");
+  }
+
   @Test
   public void testDownloadWithStdoutStderrOnSuccess() throws Exception {
     // Tests that fetching stdout/stderr as a digest works and that OutErr is still