Inconsistent INTERNAL errors for message-less RPCs

[jhspaybar]

What version of gRPC are you using?

1.0.3

What JVM are you using (java -version)?

java version "1.8.0_121"

What did you do?

Create a unary rpc, on the server have it immediately call responseObserver.onCompleted() with no previous onNext(). With a java gRPC client call this rpc with blocking and future stubs, they'll both throw an INTERNAL error from here

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

Line 434 in 89bc2cd

Status.INTERNAL.withDescription("No value received for unary call")

. Using the async client stubs will work(no onNext, just onCompleted with trailers).

If I switched to a streaming response and called onComplete() on with no previous onNext() the future stub is no longer available(expected) and now the Iterable for the blocking stub returns false for hasNext() as expected, but doesn't break. The async stub continues working as before.

What did you expect to see?

I guess I wanted the "defaultInstance" of the response type for the blocking and future based stubs rather than an exception. That, or an onError to trigger on the async stub with the same INTERNAL status and message.

I have not checked what the c-core will do in this case, or if there is a spec for this behavior.

[jhspaybar]

@ejona86 I see you had a similar issue here grpc/grpc#8010 which makes it sound like servers should always call onNext() for a unary response, but don't need to for streaming responses?

[jhspaybar]

related #2770

[ejona86]

A "stream" is zero-to-many messages, so no call to onNext() is valid. Unary must either error or have a message.

It seems like there are two issues:

1. The "unary must have one message" check is implemented in the stub, instead of ClientCallImpl. I think there might have been something small that made this slightly harder than trivial to fix. But it should be pretty easy.
2. The server does not enforce "unary must have one message." If the application closes with OK but no messages, this should become an error of some sort (sent to the client). This would ideally be done in ServerCallImpl.

@jhspaybar, does that sound okay?

[ejona86]

See also #2243. This is sort of a dup, and sort of not. Leaving open for the moment.

[hsaliak]

@ejona86 another one for us to close..

[ejona86]

Yeah, I think @zpencer fixed this case at the same time as #2243.