grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

This reverts commit 99f86835ed.

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.
This commit is contained in:
Eric Anderson 2024-10-27 21:29:45 -07:00 committed by GitHub
parent 2d0c158987
commit 135f433bcd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 4 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
stub/src/main/java/io/grpc/stub/ServerCalls.java
View File

@ -335,7 +335,6 @@ public final class ServerCalls {
private boolean aborted = false; private boolean aborted = false;
private boolean completed = false; private boolean completed = false;
private Runnable onCloseHandler; private Runnable onCloseHandler;
private RespT unaryResponse;
// Non private to avoid synthetic class // Non private to avoid synthetic class
ServerCallStreamObserverImpl(ServerCall<ReqT, RespT> call, boolean serverStreamingOrBidi) { ServerCallStreamObserverImpl(ServerCall<ReqT, RespT> call, boolean serverStreamingOrBidi) {
@ -374,22 +373,15 @@ public final class ServerCalls {
} }
checkState(!aborted, "Stream was terminated by error, no further calls are allowed"); checkState(!aborted, "Stream was terminated by error, no further calls are allowed");
checkState(!completed, "Stream is already completed, no further calls are allowed"); checkState(!completed, "Stream is already completed, no further calls are allowed");
if (serverStreamingOrBidi) {
if (!sentHeaders) { if (!sentHeaders) {
call.sendHeaders(new Metadata()); call.sendHeaders(new Metadata());
sentHeaders = true; sentHeaders = true;
} }
call.sendMessage(response); call.sendMessage(response);
} else {
unaryResponse = response;
}
} }
@Override @Override
public void onError(Throwable t) { public void onError(Throwable t) {
if (!serverStreamingOrBidi) {
unaryResponse = null;
}
Metadata metadata = Status.trailersFromThrowable(t); Metadata metadata = Status.trailersFromThrowable(t);
if (metadata == null) { if (metadata == null) {
metadata = new Metadata(); metadata = new Metadata();
@ -400,14 +392,6 @@ public final class ServerCalls {
@Override @Override
public void onCompleted() { 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()); call.close(Status.OK, new Metadata());
completed = true; completed = true;
} }

54
stub/src/test/java/io/grpc/stub/ServerCallsTest.java
View File

@ -33,7 +33,6 @@ import io.grpc.ServerCallHandler;
import io.grpc.ServerServiceDefinition; import io.grpc.ServerServiceDefinition;
import io.grpc.ServiceDescriptor; import io.grpc.ServiceDescriptor;
import io.grpc.Status; import io.grpc.Status;
import io.grpc.Status.Code;
import io.grpc.StatusRuntimeException; import io.grpc.StatusRuntimeException;
import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessChannelBuilder;
import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.inprocess.InProcessServerBuilder;
@ -621,59 +620,6 @@ public class ServerCallsTest {
assertArrayEquals(new int[]{0, 1, 1, 2, 2, 2}, receivedMessages); 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> { public static class IntegerMarshaller implements MethodDescriptor.Marshaller<Integer> {
@Override @Override
public InputStream stream(Integer value) { public InputStream stream(Integer value) {