commit 5122617b8a22fee7acd86c9c48f2c2737709ca3f
author George Gensure <werkt0@gmail.com> Tue Dec 01 01:33:42 2020 -0800
committer Copybara-Service <copybara-worker@google.com> Tue Dec 01 01:34:50 2020 -0800
tree 75fb3fe49757d5ac0059931ef795f4de74a3355e
parent 12b655b582c0d4eee5d7f62da62957336557cd96

Status error presentation with details

Remote Execution Status messages embedded in ExecuteResponses are
extremely capable vehicles for conveying the nature of an error, and
informing a user of further steps to take to remediate it. This change
expands the presentation of these response Statuses, and brings all of
the error details to light, by default instead of requiring
--verbose_failures to investigate any details of a remote execution
problem.

The interpretation of precondition failures to highlight retriable
responses has been expanded to ignore benign details that might be
included in a response.

SpawnResult error message composition has been simplified substantially,
without any special behavior for 'Remote' errors, and a removal of a
duplicate message printout incurred in the wake of succcessive @janakr
and @olaola changes. Failure messages are now implied to be present in
all spawn result failure reporting exactly once, and the failureMessage
of a SpawnResult is implied to be the parameter to getDetailMessage.

An example error presentation is as follows (including the modifications
to SpawnResult's output formatting):

```
ERROR: /home/werkt/dev/test/BUILD:22:10: Linking test failed: (Exit 34): Remote Execution Failure:
Failed Precondition: Action 4223ab2cc114385110714243a0b4a88cc743f2169b5be7d4d438a6bbba4f529f/142 is invalid
  Resource Info: type.googleapis.com/google.longrunning.Operation: name='shard/operations/9335fef2-184b-4d26-9a6f-2f27cebe7527', owner='tool_invocation_id:4b4bf7b1-fadd-44fd-99be-a234e7c26fc4,correlated_invocation_id:dc88325a-9317-48c0-9013-b3bb8b7a458f'
  Precondition Failure:
    (MISSING) bazel-out/k8-fastbuild/bin/test: 7872: An output could not be uploaded because it exceeded the maximum size of an entry
Target //:test failed to build
```

Closes #12564.

PiperOrigin-RevId: 344973841

src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
index 247ee8a..eec073a 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java
@@ -17,6 +17,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
 
 import build.bazel.remote.execution.v2.Digest;
 import build.bazel.remote.execution.v2.RequestMetadata;
@@ -330,7 +331,7 @@
     uploader.uploadBlob(hash, chunker, true);
 
     // This test should not have triggered any retries.
-    Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis();
+    Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class));
     Mockito.verify(mockBackoff, Mockito.times(1)).getRetryAttempts();
 
     blockUntilInternalStateConsistent(uploader);
@@ -469,7 +470,7 @@
 
     // This test should have triggered a single retry, because it made
     // no progress.
-    Mockito.verify(mockBackoff, Mockito.times(1)).nextDelayMillis();
+    Mockito.verify(mockBackoff, Mockito.times(1)).nextDelayMillis(any(Exception.class));
 
     blockUntilInternalStateConsistent(uploader);
 
@@ -1402,7 +1403,7 @@
     }
 
     @Override
-    public long nextDelayMillis() {
+    public long nextDelayMillis(Exception e) {
       if (retries < maxRetries) {
         retries++;
         return delayMillis;