From 258a95d0c4451a2ca1055358cb41d09c034ee6b4 Mon Sep 17 00:00:00 2001
From: Ran <ran-su@users.noreply.github.com>
Date: Mon, 28 Oct 2019 10:05:28 -0700
Subject: [PATCH] Revert "stub: ignore unary response msg if status is not OK
 (#6288)" (#6342)

This reverts commit fe46edac
---
 .../main/java/io/grpc/stub/ClientCalls.java   |  29 +----
 .../java/io/grpc/stub/ClientCallsTest.java    | 117 ------------------
 2 files changed, 2 insertions(+), 144 deletions(-)

diff --git a/stub/src/main/java/io/grpc/stub/ClientCalls.java b/stub/src/main/java/io/grpc/stub/ClientCalls.java
index 7240e122d2..cf770a991e 100644
--- a/stub/src/main/java/io/grpc/stub/ClientCalls.java
+++ b/stub/src/main/java/io/grpc/stub/ClientCalls.java
@@ -401,7 +401,6 @@ public final class ClientCalls {
     private final CallToStreamObserverAdapter<ReqT> adapter;
     private final boolean streamingResponse;
     private boolean firstResponseReceived;
-    private RespT unaryMessage;
 
     // Non private to avoid synthetic class
     StreamObserverToCallListenerAdapter(
@@ -432,13 +431,7 @@ public final class ClientCalls {
             .asRuntimeException();
       }
       firstResponseReceived = true;
-
-      if (streamingResponse) {
-        observer.onNext(message);
-      } else {
-        // will send message in onClose() for unary calls.
-        unaryMessage = message;
-      }
+      observer.onNext(message);
 
       if (streamingResponse && adapter.autoFlowControlEnabled) {
         // Request delivery of the next inbound message.
@@ -448,28 +441,10 @@ public final class ClientCalls {
 
     @Override
     public void onClose(Status status, Metadata trailers) {
-      Throwable error = null;
       if (status.isOk()) {
-        if (!streamingResponse) {
-          if (unaryMessage != null) {
-            try {
-              observer.onNext(unaryMessage);
-            } catch (Throwable t) {
-              error = t;
-            }
-          } else {
-            error = Status.INTERNAL.withDescription("Response message is null for unary call")
-                .asRuntimeException();
-          }
-        }
-      } else {
-        error = status.asRuntimeException(trailers);
-      }
-
-      if (error == null) {
         observer.onCompleted();
       } else {
-        observer.onError(error);
+        observer.onError(status.asRuntimeException(trailers));
       }
     }
 
diff --git a/stub/src/test/java/io/grpc/stub/ClientCallsTest.java b/stub/src/test/java/io/grpc/stub/ClientCallsTest.java
index bcae5340a2..a636487706 100644
--- a/stub/src/test/java/io/grpc/stub/ClientCallsTest.java
+++ b/stub/src/test/java/io/grpc/stub/ClientCallsTest.java
@@ -99,123 +99,6 @@ public class ClientCallsTest {
     }
   }
 
-  @Test
-  public void unaryAsyncCallStatusIsOkWithMessageSuccess() throws Exception {
-    Integer req = 2;
-    final String resp = "bar";
-    final Status status = Status.OK;
-    final Metadata trailers = new Metadata();
-    final List<String> actualResponse = new ArrayList<>();
-    final List<Boolean> completed = new ArrayList<>();
-
-    NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
-      @Override
-      public void start(ClientCall.Listener<String> listener, Metadata headers) {
-        listener.onMessage(resp);
-        listener.onClose(status, trailers);
-      }
-    };
-
-    StreamObserver<String> responseObserver = new StreamObserver<String>() {
-      @Override
-      public void onNext(String value) {
-        actualResponse.add(value);
-      }
-
-      @Override
-      public void onError(Throwable t) {
-        fail("Should not reach here");
-      }
-
-      @Override
-      public void onCompleted() {
-        completed.add(true);
-      }
-    };
-
-    ClientCalls.asyncUnaryCall(call, req, responseObserver);
-    assertThat(actualResponse).hasSize(1);
-    assertEquals(resp, actualResponse.get(0));
-    assertThat(completed).hasSize(1);
-    assertThat(completed.get(0)).isTrue();
-  }
-
-  @Test
-  public void unaryAsyncCallStatusIsOkWithNullMessageGetError() throws Exception {
-    Integer req = 2;
-    final Status status = Status.OK;
-    final Metadata trailers = new Metadata();
-    final List<Throwable> expected = new ArrayList<>();
-
-    NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
-      @Override
-      public void start(ClientCall.Listener<String> listener, Metadata headers) {
-        listener.onMessage(null);
-        listener.onClose(status, trailers);
-      }
-    };
-
-    StreamObserver<String> responseObserver = new StreamObserver<String>() {
-      @Override
-      public void onNext(String value) {
-        fail("Should not reach here");
-      }
-
-      @Override
-      public void onError(Throwable t) {
-        expected.add(t);
-      }
-
-      @Override
-      public void onCompleted() {
-        fail("Should not reach here");
-      }
-    };
-
-    ClientCalls.asyncUnaryCall(call, req, responseObserver);
-    assertThat(expected).hasSize(1);
-    assertThat(expected.get(0)).hasMessageThat()
-        .isEqualTo("INTERNAL: Response message is null for unary call");
-  }
-
-  @Test
-  public void unaryAsyncCallStatusIsNotOkWithMessageDoNotSendMessage() throws Exception {
-    Integer req = 2;
-    final Status status = Status.INTERNAL.withDescription("Unique status");
-    final String resp = "bar";
-    final Metadata trailers = new Metadata();
-    final List<Throwable> expected = new ArrayList<>();
-
-    NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
-      @Override
-      public void start(io.grpc.ClientCall.Listener<String> listener, Metadata headers) {
-        listener.onMessage(resp);
-        listener.onClose(status, trailers);
-      }
-    };
-
-    StreamObserver<String> responseObserver = new StreamObserver<String>() {
-      @Override
-      public void onNext(String value) {
-        fail("Should not reach here");
-      }
-
-      @Override
-      public void onError(Throwable t) {
-        expected.add(t);
-      }
-
-      @Override
-      public void onCompleted() {
-        fail("Should not reach here");
-      }
-    };
-
-    ClientCalls.asyncUnaryCall(call, req, responseObserver);
-    assertThat(expected).hasSize(1);
-    assertThat(expected.get(0)).hasMessageThat().isEqualTo("INTERNAL: Unique status");
-  }
-
   @Test
   public void unaryBlockingCallSuccess() throws Exception {
     Integer req = 2;