Operation stream termination is not an error

According to the GrpcRemoteExecutor when it occurs after a !done
operation response. Remove the error from the
ExperimentalRemoteGrpcExecutor and reinforce both with tests.

Update the FakeExecutionService to generate compatible error responses
that appear in the ExecuteResponse, rather than the operation error
field, per the REAPI spec. Made required adjustments to ExGRE Test
invocations to avoid the ExecutionStatusException interpretation of
DEADLINE_EXCEEDED -> FAILED_PRECONDITION in ExecuteResponse.

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

@@ -146,10 +146,12 @@ ExecuteResponse start() throws IOException, InterruptedException {
  // retrying when received a unauthenticated error, and propagate to refreshIfUnauthenticated
  // which will then call retrier again. It will reset the retry time counter so we could
  // retry more than --remote_retry times which is not expected.
- response =
- retrier.execute(
- () -> Utils.refreshIfUnauthenticated(this::execute, callCredentialsProvider),
- executeBackoff);
+ if (lastOperation == null) {
+ response =
+ retrier.execute(
+ () -> Utils.refreshIfUnauthenticated(this::execute, callCredentialsProvider),
+ executeBackoff);
+ }
 
  // If no response from Execute(), use WaitExecution() in a "loop" which is implemented
  // inside the retry block.
@@ -258,8 +260,8 @@ ExecuteResponse handleOperationStream(Iterator<Operation> operationStream) throw
  }
  }
 
- // The operation completed successfully but without a result.
- throw new IOException("Remote server error: execution terminated with no result");
+ // The operation stream completed successfully but without a result.
+ return null;
  } finally {
  close(operationStream);
  }

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

@@ -265,10 +265,10 @@ public void executeRemotely_executeAndRetryWait() throws Exception {
 
  @Test
  public void executeRemotely_executeAndRetryWait_forever() throws Exception {
- executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE);
+ executionService.whenExecute(DUMMY_REQUEST).thenAck().finish();
  int errorTimes = MAX_RETRY_ATTEMPTS * 2;
  for (int i = 0; i < errorTimes; ++i) {
- executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenError(Code.DEADLINE_EXCEEDED);
+ executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenError(Status.DEADLINE_EXCEEDED.asRuntimeException());
  }
  executionService.whenWaitExecution(DUMMY_REQUEST).thenDone(DUMMY_RESPONSE);
 
@@ -282,7 +282,7 @@ public void executeRemotely_executeAndRetryWait_forever() throws Exception {
 
  @Test
  public void executeRemotely_executeAndRetryWait_failForConsecutiveErrors() {
- executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE);
+ executionService.whenExecute(DUMMY_REQUEST).thenAck().finish();
  for (int i = 0; i < MAX_RETRY_ATTEMPTS * 2; ++i) {
  executionService.whenWaitExecution(DUMMY_REQUEST).thenError(Code.UNAVAILABLE);
  }
@@ -356,7 +356,7 @@ public void executeRemotely_retryExecuteWhenUnauthenticated()
  @Test
  public void executeRemotely_retryWaitExecutionWhenUnauthenticated()
  throws IOException, InterruptedException {
- executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE);
+ executionService.whenExecute(DUMMY_REQUEST).thenAck().finish();
  executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenError(Code.UNAUTHENTICATED);
  executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE);
 

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

@@ -115,12 +115,14 @@ public void thenDone(ExecuteResponse response) {
  }
 
  public void thenError(Code code) {
- Operation operation =
- Operation.newBuilder()
- .setName(getResourceName(request))
- .setDone(true)
- .setError(Status.newBuilder().setCode(code.getNumber()))
- .build();
+ // From REAPI Spec:
+ // > Errors discovered during creation of the `Operation` will be reported
+ // > as gRPC Status errors, while errors that occurred while running the
+ // > action will be reported in the `status` field of the `ExecuteResponse`. The
+ // > server MUST NOT set the `error` field of the `Operation` proto.
+ Operation operation = doneOperation(request, ExecuteResponse.newBuilder()
+ .setStatus(Status.newBuilder().setCode(code.getNumber()))
+ .build());
  operations.add(() -> operation);
  finish();
  }
@@ -133,7 +135,7 @@ public void thenError(RuntimeException e) {
  finish();
  }
 
- private void finish() {
+ public void finish() {
  String name = getResourceName(request);
  provider.append(name, ImmutableList.copyOf(operations));
  }