Skip to content

Commit 99f8683

Browse files
stub: Ignore unary response on server if status is not OK
Fixes grpc#5969
1 parent 36e29ab commit 99f8683

File tree

2 files changed

+74
-4
lines changed

2 files changed

+74
-4
lines changed

stub/src/main/java/io/grpc/stub/ServerCalls.java

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ private static final class ServerCallStreamObserverImpl<ReqT, RespT>
335335
private boolean aborted = false;
336336
private boolean completed = false;
337337
private Runnable onCloseHandler;
338+
private RespT unaryResponse;
338339

339340
// Non private to avoid synthetic class
340341
ServerCallStreamObserverImpl(ServerCall<ReqT, RespT> call, boolean serverStreamingOrBidi) {
@@ -373,15 +374,22 @@ public void onNext(RespT response) {
373374
}
374375
checkState(!aborted, "Stream was terminated by error, no further calls are allowed");
375376
checkState(!completed, "Stream is already completed, no further calls are allowed");
376-
if (!sentHeaders) {
377-
call.sendHeaders(new Metadata());
378-
sentHeaders = true;
377+
if (serverStreamingOrBidi) {
378+
if (!sentHeaders) {
379+
call.sendHeaders(new Metadata());
380+
sentHeaders = true;
381+
}
382+
call.sendMessage(response);
383+
} else {
384+
unaryResponse = response;
379385
}
380-
call.sendMessage(response);
381386
}
382387

383388
@Override
384389
public void onError(Throwable t) {
390+
if (!serverStreamingOrBidi) {
391+
unaryResponse = null;
392+
}
385393
Metadata metadata = Status.trailersFromThrowable(t);
386394
if (metadata == null) {
387395
metadata = new Metadata();
@@ -392,6 +400,14 @@ public void onError(Throwable t) {
392400

393401
@Override
394402
public void onCompleted() {
403+
if (!serverStreamingOrBidi && unaryResponse != null) {
404+
if (!sentHeaders) {
405+
call.sendHeaders(new Metadata());
406+
sentHeaders = true;
407+
}
408+
call.sendMessage(unaryResponse);
409+
unaryResponse = null;
410+
}
395411
call.close(Status.OK, new Metadata());
396412
completed = true;
397413
}

stub/src/test/java/io/grpc/stub/ServerCallsTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import io.grpc.ServerServiceDefinition;
3434
import io.grpc.ServiceDescriptor;
3535
import io.grpc.Status;
36+
import io.grpc.Status.Code;
3637
import io.grpc.StatusRuntimeException;
3738
import io.grpc.inprocess.InProcessChannelBuilder;
3839
import io.grpc.inprocess.InProcessServerBuilder;
@@ -620,6 +621,59 @@ public void onClose(Status status, Metadata trailers) {
620621
assertArrayEquals(new int[]{0, 1, 1, 2, 2, 2}, receivedMessages);
621622
}
622623

624+
@Test
625+
public void serverUnaryResponseMsgWithOkStatus() {
626+
ServerCallHandler<Integer, Integer> serverCallHandler =
627+
ServerCalls.asyncUnaryCall(
628+
new ServerCalls.UnaryMethod<Integer, Integer>() {
629+
@Override
630+
public void invoke(Integer request, StreamObserver<Integer> responseObserver) {
631+
responseObserver.onNext(request);
632+
responseObserver.onCompleted();
633+
}
634+
});
635+
ServerCall.Listener<Integer> callListener =
636+
serverCallHandler.startCall(serverCall, new Metadata());
637+
serverCall.isReady = true;
638+
serverCall.isCancelled = false;
639+
callListener.onReady();
640+
callListener.onMessage(1);
641+
callListener.onHalfClose();
642+
643+
assertThat(serverCall.status.getCode()).isEqualTo(Code.OK);
644+
assertThat(serverCall.responses).containsExactly(1);
645+
}
646+
647+
@Test
648+
public void serverUnaryResponseMsgWithNotOkStatus() {
649+
ServerCallHandler<Integer, Integer> serverCallHandler =
650+
ServerCalls.asyncUnaryCall(
651+
new ServerCalls.UnaryMethod<Integer, Integer>() {
652+
@Override
653+
public void invoke(Integer request, StreamObserver<Integer> responseObserver) {
654+
responseObserver.onNext(request);
655+
responseObserver.onError(
656+
Status.INTERNAL
657+
.withDescription("Response message is null for unary call")
658+
.asRuntimeException());
659+
}
660+
});
661+
662+
ServerCall.Listener<Integer> callListener =
663+
serverCallHandler.startCall(serverCall, new Metadata());
664+
665+
serverCall.isReady = true;
666+
serverCall.isCancelled = false;
667+
callListener.onReady();
668+
callListener.onMessage(1);
669+
callListener.onHalfClose();
670+
671+
assertThat(serverCall.status.getCode()).isEqualTo(Code.INTERNAL);
672+
assertThat(serverCall.status.getDescription())
673+
.isEqualTo("Response message is null for unary call");
674+
assertThat(serverCall.responses).isEmpty();
675+
}
676+
623677
public static class IntegerMarshaller implements MethodDescriptor.Marshaller<Integer> {
624678
@Override
625679
public InputStream stream(Integer value) {

0 commit comments

Comments
 (0)