Skip to content

Commit 7ccea99

Browse files
authored
Revert "stub: ignore unary response msg if status is not OK (#6288)" (#6342) (#6351)
This reverts commit fe46eda (cherry picked from commit 258a95d)
1 parent 38952b0 commit 7ccea99

File tree

2 files changed

+2
-144
lines changed

2 files changed

+2
-144
lines changed

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

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,6 @@ private static final class StreamObserverToCallListenerAdapter<ReqT, RespT>
401401
private final CallToStreamObserverAdapter<ReqT> adapter;
402402
private final boolean streamingResponse;
403403
private boolean firstResponseReceived;
404-
private RespT unaryMessage;
405404

406405
// Non private to avoid synthetic class
407406
StreamObserverToCallListenerAdapter(
@@ -432,13 +431,7 @@ public void onMessage(RespT message) {
432431
.asRuntimeException();
433432
}
434433
firstResponseReceived = true;
435-
436-
if (streamingResponse) {
437-
observer.onNext(message);
438-
} else {
439-
// will send message in onClose() for unary calls.
440-
unaryMessage = message;
441-
}
434+
observer.onNext(message);
442435

443436
if (streamingResponse && adapter.autoFlowControlEnabled) {
444437
// Request delivery of the next inbound message.
@@ -448,28 +441,10 @@ public void onMessage(RespT message) {
448441

449442
@Override
450443
public void onClose(Status status, Metadata trailers) {
451-
Throwable error = null;
452444
if (status.isOk()) {
453-
if (!streamingResponse) {
454-
if (unaryMessage != null) {
455-
try {
456-
observer.onNext(unaryMessage);
457-
} catch (Throwable t) {
458-
error = t;
459-
}
460-
} else {
461-
error = Status.INTERNAL.withDescription("Response message is null for unary call")
462-
.asRuntimeException();
463-
}
464-
}
465-
} else {
466-
error = status.asRuntimeException(trailers);
467-
}
468-
469-
if (error == null) {
470445
observer.onCompleted();
471446
} else {
472-
observer.onError(error);
447+
observer.onError(status.asRuntimeException(trailers));
473448
}
474449
}
475450

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

Lines changed: 0 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -99,123 +99,6 @@ public void tearDown() {
9999
}
100100
}
101101

102-
@Test
103-
public void unaryAsyncCallStatusIsOkWithMessageSuccess() throws Exception {
104-
Integer req = 2;
105-
final String resp = "bar";
106-
final Status status = Status.OK;
107-
final Metadata trailers = new Metadata();
108-
final List<String> actualResponse = new ArrayList<>();
109-
final List<Boolean> completed = new ArrayList<>();
110-
111-
NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
112-
@Override
113-
public void start(ClientCall.Listener<String> listener, Metadata headers) {
114-
listener.onMessage(resp);
115-
listener.onClose(status, trailers);
116-
}
117-
};
118-
119-
StreamObserver<String> responseObserver = new StreamObserver<String>() {
120-
@Override
121-
public void onNext(String value) {
122-
actualResponse.add(value);
123-
}
124-
125-
@Override
126-
public void onError(Throwable t) {
127-
fail("Should not reach here");
128-
}
129-
130-
@Override
131-
public void onCompleted() {
132-
completed.add(true);
133-
}
134-
};
135-
136-
ClientCalls.asyncUnaryCall(call, req, responseObserver);
137-
assertThat(actualResponse).hasSize(1);
138-
assertEquals(resp, actualResponse.get(0));
139-
assertThat(completed).hasSize(1);
140-
assertThat(completed.get(0)).isTrue();
141-
}
142-
143-
@Test
144-
public void unaryAsyncCallStatusIsOkWithNullMessageGetError() throws Exception {
145-
Integer req = 2;
146-
final Status status = Status.OK;
147-
final Metadata trailers = new Metadata();
148-
final List<Throwable> expected = new ArrayList<>();
149-
150-
NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
151-
@Override
152-
public void start(ClientCall.Listener<String> listener, Metadata headers) {
153-
listener.onMessage(null);
154-
listener.onClose(status, trailers);
155-
}
156-
};
157-
158-
StreamObserver<String> responseObserver = new StreamObserver<String>() {
159-
@Override
160-
public void onNext(String value) {
161-
fail("Should not reach here");
162-
}
163-
164-
@Override
165-
public void onError(Throwable t) {
166-
expected.add(t);
167-
}
168-
169-
@Override
170-
public void onCompleted() {
171-
fail("Should not reach here");
172-
}
173-
};
174-
175-
ClientCalls.asyncUnaryCall(call, req, responseObserver);
176-
assertThat(expected).hasSize(1);
177-
assertThat(expected.get(0)).hasMessageThat()
178-
.isEqualTo("INTERNAL: Response message is null for unary call");
179-
}
180-
181-
@Test
182-
public void unaryAsyncCallStatusIsNotOkWithMessageDoNotSendMessage() throws Exception {
183-
Integer req = 2;
184-
final Status status = Status.INTERNAL.withDescription("Unique status");
185-
final String resp = "bar";
186-
final Metadata trailers = new Metadata();
187-
final List<Throwable> expected = new ArrayList<>();
188-
189-
NoopClientCall<Integer, String> call = new NoopClientCall<Integer, String>() {
190-
@Override
191-
public void start(io.grpc.ClientCall.Listener<String> listener, Metadata headers) {
192-
listener.onMessage(resp);
193-
listener.onClose(status, trailers);
194-
}
195-
};
196-
197-
StreamObserver<String> responseObserver = new StreamObserver<String>() {
198-
@Override
199-
public void onNext(String value) {
200-
fail("Should not reach here");
201-
}
202-
203-
@Override
204-
public void onError(Throwable t) {
205-
expected.add(t);
206-
}
207-
208-
@Override
209-
public void onCompleted() {
210-
fail("Should not reach here");
211-
}
212-
};
213-
214-
ClientCalls.asyncUnaryCall(call, req, responseObserver);
215-
assertThat(expected).hasSize(1);
216-
assertThat(expected.get(0)).hasMessageThat().isEqualTo("INTERNAL: Unique status");
217-
}
218-
219102
@Test
220103
public void unaryBlockingCallSuccess() throws Exception {
221104
Integer req = 2;

0 commit comments

Comments
 (0)