Status error presentation with details#12564
Closed
werkt wants to merge 3 commits intobazelbuild:masterfrom
Closed
Status error presentation with details#12564werkt wants to merge 3 commits intobazelbuild:masterfrom
werkt wants to merge 3 commits intobazelbuild:masterfrom
Conversation
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.
werkt
force-pushed
the
remote-error-details
branch
from
71e3903 to
551bc7a
Compare
coeuvre
reviewed
Nov 30, 2020
| import io.grpc.Status.Code; | ||
| import io.grpc.protobuf.StatusProto; | ||
|
|
||
| class ExecuteRetrier extends RemoteRetrier { |
Contributor
Author
There was a problem hiding this comment.
Hope a single line is sufficient here, didn't see much else on the other classes I looked at.
| for (Any detail : status.getDetailsList()) { | ||
| if (detail.is(RetryInfo.class)) { | ||
| try { | ||
| retryInfo = detail.unpack(RetryInfo.class); |
Contributor
Author
There was a problem hiding this comment.
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.
| for (Any detail : status.getDetailsList()) { | ||
| if (detail.is(RetryInfo.class)) { | ||
| // server says we can retry, regardless of other details | ||
| fullyRetriable = true; |
Contributor
Author
There was a problem hiding this comment.
Early return here - nothing after it is effective, and coming up with whether the precondition failure is effective is meaningless. Thanks!
coeuvre
approved these changes
Dec 1, 2020
philwo
pushed a commit
that referenced
this pull request
Mar 15, 2021
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
philwo
pushed a commit
that referenced
this pull request
Mar 15, 2021
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):