Revert "stub: Ignore unary response on server if status is not OK" (#11636) (#11640)

ejona86 · web-flow · commit 135f433bcd21 · 2024-10-28T09:59:45.000+05:30
This reverts commit 99f8683. The change doesn't handle `null` messages, which don't happen with protobuf, but can happen with other marshallers, especially in tests. See cl/689445172 This will reopen #5969.
diff --git a/stub/src/main/java/io/grpc/stub/ServerCalls.java b/stub/src/main/java/io/grpc/stub/ServerCalls.java
@@ -335,7 +335,6 @@ private static final class ServerCallStreamObserverImpl<ReqT, RespT>
     private boolean aborted = false;
     private boolean completed = false;
     private Runnable onCloseHandler;
-    private RespT unaryResponse;
 
     // Non private to avoid synthetic class
     ServerCallStreamObserverImpl(ServerCall<ReqT, RespT> call, boolean serverStreamingOrBidi) {
@@ -374,22 +373,15 @@ public void onNext(RespT response) {
       }
       checkState(!aborted, "Stream was terminated by error, no further calls are allowed");
       checkState(!completed, "Stream is already completed, no further calls are allowed");
-      if (serverStreamingOrBidi) {
-        if (!sentHeaders) {
-          call.sendHeaders(new Metadata());
-          sentHeaders = true;
-        }
-        call.sendMessage(response);
-      } else {
-        unaryResponse = response;
+      if (!sentHeaders) {
+        call.sendHeaders(new Metadata());
+        sentHeaders = true;
       }
+      call.sendMessage(response);
     }
 
     @Override
     public void onError(Throwable t) {
-      if (!serverStreamingOrBidi) {
-        unaryResponse = null;
-      }
       Metadata metadata = Status.trailersFromThrowable(t);
       if (metadata == null) {
         metadata = new Metadata();
@@ -400,14 +392,6 @@ public void onError(Throwable t) {
 
     @Override
     public void onCompleted() {
-      if (!serverStreamingOrBidi && unaryResponse != null) {
-        if (!sentHeaders) {
-          call.sendHeaders(new Metadata());
-          sentHeaders = true;
-        }
-        call.sendMessage(unaryResponse);
-        unaryResponse = null;
-      }
       call.close(Status.OK, new Metadata());
       completed = true;
     }
diff --git a/stub/src/test/java/io/grpc/stub/ServerCallsTest.java b/stub/src/test/java/io/grpc/stub/ServerCallsTest.java
@@ -33,7 +33,6 @@
 import io.grpc.ServerServiceDefinition;
 import io.grpc.ServiceDescriptor;
 import io.grpc.Status;
-import io.grpc.Status.Code;
 import io.grpc.StatusRuntimeException;
 import io.grpc.inprocess.InProcessChannelBuilder;
 import io.grpc.inprocess.InProcessServerBuilder;
@@ -621,59 +620,6 @@ public void onClose(Status status, Metadata trailers) {
     assertArrayEquals(new int[]{0, 1, 1, 2, 2, 2}, receivedMessages);
   }
 
-  @Test
-  public void serverUnaryResponseMsgWithOkStatus() {
-    ServerCallHandler<Integer, Integer> serverCallHandler =
-        ServerCalls.asyncUnaryCall(
-            new ServerCalls.UnaryMethod<Integer, Integer>() {
-              @Override
-              public void invoke(Integer request, StreamObserver<Integer> responseObserver) {
-                responseObserver.onNext(request);
-                responseObserver.onCompleted();
-              }
-            });
-    ServerCall.Listener<Integer> callListener =
-        serverCallHandler.startCall(serverCall, new Metadata());
-    serverCall.isReady = true;
-    serverCall.isCancelled = false;
-    callListener.onReady();
-    callListener.onMessage(1);
-    callListener.onHalfClose();
-
-    assertThat(serverCall.status.getCode()).isEqualTo(Code.OK);
-    assertThat(serverCall.responses).containsExactly(1);
-  }
-
-  @Test
-  public void serverUnaryResponseMsgWithNotOkStatus() {
-    ServerCallHandler<Integer, Integer> serverCallHandler =
-        ServerCalls.asyncUnaryCall(
-            new ServerCalls.UnaryMethod<Integer, Integer>() {
-              @Override
-              public void invoke(Integer request, StreamObserver<Integer> responseObserver) {
-                responseObserver.onNext(request);
-                responseObserver.onError(
-                    Status.INTERNAL
-                        .withDescription("Response message is null for unary call")
-                        .asRuntimeException());
-              }
-            });
-
-    ServerCall.Listener<Integer> callListener =
-        serverCallHandler.startCall(serverCall, new Metadata());
-
-    serverCall.isReady = true;
-    serverCall.isCancelled = false;
-    callListener.onReady();
-    callListener.onMessage(1);
-    callListener.onHalfClose();
-
-    assertThat(serverCall.status.getCode()).isEqualTo(Code.INTERNAL);
-    assertThat(serverCall.status.getDescription())
-        .isEqualTo("Response message is null for unary call");
-    assertThat(serverCall.responses).isEmpty();
-  }
-
   public static class IntegerMarshaller implements MethodDescriptor.Marshaller<Integer> {
     @Override
     public InputStream stream(Integer value) {
