Status error presentation with details #12564

werkt · 2020-11-26T07:36:24Z

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

@janakr

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 ```

When a retryInfo is supplied, it should circumvent any other conditions which would prevent retriability. Its delay will inform the subsequent backoff delay supplied, assuming it is not beyond the retry count.

coeuvre

Thanks!

coeuvre · 2020-11-30T04:53:32Z

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

+import io.grpc.Status.Code;
+import io.grpc.protobuf.StatusProto;
+
+class ExecuteRetrier extends RemoteRetrier {


nit: please add javadoc

Hope a single line is sufficient here, didn't see much else on the other classes I looked at.

coeuvre · 2020-11-30T04:55:12Z

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

+        for (Any detail : status.getDetailsList()) {
+          if (detail.is(RetryInfo.class)) {
+            try {
+              retryInfo = detail.unpack(RetryInfo.class);


break here?

I want the retryInfo to have a deterministic last-specified behavior. Not that I want to see multiple from the service, but if it does, the last one in the list should be effective.

coeuvre · 2020-11-30T05:25:17Z

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

+    for (Any detail : status.getDetailsList()) {
+      if (detail.is(RetryInfo.class)) {
+        // server says we can retry, regardless of other details
+        fullyRetriable = true;


break here?

Early return here - nothing after it is effective, and coming up with whether the precondition failure is effective is meaningless. Thanks!

@janakr

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: 3449738

@janakr

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: 3449738

google-cla bot added the cla: yes label Nov 26, 2020

werkt mentioned this pull request Nov 26, 2020

FAILED_PRECONDITION: duplicate files buildfarm/buildfarm#603

Closed

Make the Execute retrier derive retry delay

551bc7a

When a retryInfo is supplied, it should circumvent any other conditions which would prevent retriability. Its delay will inform the subsequent backoff delay supplied, assuming it is not beyond the retry count.

werkt force-pushed the remote-error-details branch from 71e3903 to 551bc7a Compare November 29, 2020 17:24

aiuto added the team-Remote-Exec Issues and PRs for the Execution (Remote) team label Nov 30, 2020

coeuvre reviewed Nov 30, 2020

View reviewed changes

Added javadoc, cleared early return, renamed test

4aa5d5f

coeuvre approved these changes Dec 1, 2020

View reviewed changes

bazel-io closed this in 5122617 Dec 1, 2020

werkt deleted the remote-error-details branch December 1, 2020 14:21

coeuvre mentioned this pull request May 11, 2021

REv2: RPCs failing with DEADLINE_EXCEEDED no longer return a useful error message #12898

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Status error presentation with details #12564

Status error presentation with details #12564

werkt commented Nov 26, 2020

coeuvre left a comment

coeuvre Nov 30, 2020

werkt Nov 30, 2020

coeuvre Nov 30, 2020

werkt Nov 30, 2020

coeuvre Nov 30, 2020

werkt Nov 30, 2020

Status error presentation with details #12564

Status error presentation with details #12564

Conversation

werkt commented Nov 26, 2020

coeuvre left a comment

Choose a reason for hiding this comment

coeuvre Nov 30, 2020

Choose a reason for hiding this comment

werkt Nov 30, 2020

Choose a reason for hiding this comment

coeuvre Nov 30, 2020

Choose a reason for hiding this comment

werkt Nov 30, 2020

Choose a reason for hiding this comment

coeuvre Nov 30, 2020

Choose a reason for hiding this comment

werkt Nov 30, 2020

Choose a reason for hiding this comment