[8.6.0] Print name of files that fail to upload with INVALID_ARGUMENT (#28137) (#28241)

A major source of such failures are concurrent modifications, which are
more easily debugged with the filename.

Work towards #28134

Closes #28137.

PiperOrigin-RevId: 855220277
Change-Id: I378f8bad0c94fc7328c4f74fbc6fa4ac26560a4d

Commit
fcaf0ba

Co-authored-by: Fabian Meumertzheim <[email protected]>

Author: bazel-io

src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java

@@ -485,13 +485,33 @@ private void releaseOut() {
   @Override
   public ListenableFuture<Void> uploadBlob(
       RemoteActionExecutionContext context, Digest digest, Blob blob) {
-    return uploadChunker(
-        context,
-        digest,
-        Chunker.builder()
-            .setInput(digest.getSizeBytes(), blob)
-            .setCompressed(shouldCompress(digest))
-            .build());
+    return Futures.catchingAsync(
+        uploadChunker(
+            context,
+            digest,
+            Chunker.builder()
+                .setInput(digest.getSizeBytes(), blob)
+                .setCompressed(shouldCompress(digest))
+                .build()),
+        IOException.class,
+        e -> {
+          var cause = e.getCause();
+          if (!(cause instanceof StatusRuntimeException sre)) {
+            return Futures.immediateFailedFuture(e);
+          }
+          var code = sre.getStatus().getCode();
+          String blobDescription = blob.description();
+          // INVALID_ARGUMENT is returned in case of a digest mismatch, which can hint at concurrent
+          // modifications to the blob's source. Print it to help the user debug such issues.
+          // https://github.com/bazelbuild/bazel/blob/ec36eacc31678ecf4b5c25f9ab7ab166330aff28/third_party/remoteapis/build/bazel/remote/execution/v2/remote_execution.proto#L283-L286
+          if (code == Code.INVALID_ARGUMENT && blobDescription != null) {
+            return Futures.immediateFailedFuture(
+                new IOException(
+                    "while uploading %s: %s".formatted(blobDescription, e.getMessage()), e));
+          }
+          return Futures.immediateFailedFuture(e);
+        },
+        MoreExecutors.directExecutor());
   }
 
   ListenableFuture<Void> uploadChunker(

src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java

@@ -27,6 +27,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * An interface for a remote caching protocol.
@@ -124,6 +125,12 @@ interface Blob extends Closeable {
     /** Get an input stream for the blob's data. Can be called multiple times. */
     InputStream get() throws IOException;
 
+    /** An optional human-readable description of the blob's source. */
+    @Nullable
+    default String description() {
+      return null;
+    }
+
     @Override
     default void close() {}
   }
@@ -138,7 +145,20 @@ default void close() {}
    */
   default ListenableFuture<Void> uploadFile(
       RemoteActionExecutionContext context, Digest digest, Path file) {
-    return uploadBlob(context, digest, () -> new LazyFileInputStream(file));
+    return uploadBlob(
+        context,
+        digest,
+        new Blob() {
+          @Override
+          public InputStream get() {
+            return new LazyFileInputStream(file);
+          }
+
+          @Override
+          public String description() {
+            return "file " + file;
+          }
+        });
   }
 
   /**

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -3852,4 +3852,49 @@ EOF
   expect_log "The file type of 'a/dir/symlink.txt' is not supported."
 }
 
+function do_test_concurrent_modification_during_upload() {
+  mkdir -p a
+  cat > a/BUILD <<'EOF'
+genrule(
+    name = "gen_and_overwrite",
+    srcs = ["in"],
+    outs = ["out"],
+    # Non-hermetically overwrite the source file.
+    cmd = """
+cat $< $< > $@
+echo -n "overwritten" > $<
+""",
+    tags = ["local"],
+)
+
+genrule(
+    name = "remote",
+    # Forced to run after gen_and_overwrite.
+    srcs = ["in", "out"],
+    outs = ["combined"],
+    cmd = "cat $(SRCS) > $@",
+)
+EOF
+  echo -n "$1" > a/in
+
+  bazel build \
+    --remote_executor=grpc://localhost:${worker_port} \
+    //a:remote >& $TEST_log && fail "build //a:remote should fail"
+  assert_contains "overwritten" a/in
+  expect_log "while uploading file .*/a/in:"
+  expect_log_n "a/in" 1
+}
+
+function test_concurrent_modification_during_upload_shorter() {
+  do_test_concurrent_modification_during_upload "very long content that will be overwritten"
+}
+
+function test_concurrent_modification_during_upload_longer() {
+  do_test_concurrent_modification_during_upload "short"
+}
+
+function test_concurrent_modification_during_upload_same_length() {
+  do_test_concurrent_modification_during_upload "same length"
+}
+
 run_suite "Remote execution and remote cache tests"