Propagate RuntimeException's back to caller

[pongad]

When using ManagedChannelImpl, if a method of ClientCall.Listener (possibly also of ClientCall itself) throws a RuntimeException, the exception propagates up into SerializingExecutor. The executor will log it under SEVERE but cannot take corrective action. The thread initiating ClientCalls.blockingUnaryCall or its cousins will block forever waiting for a response.

Ideally, the exception should be thrown back in the thread that called blockingUnaryCall.

[carl-mastrangelo]

Could you give more detail on the problem you are having? I just looked through ClientCallImpl.ClientStreamListenerImpl, which governs access to the ClientCall.Listener. It appears that in all cases, if an exception is thrown from a user provided listener, cancel() is called. This should close down the call gracefully and eventually propagate to onClose().

[pongad]

I think it successfully propagate to onClose(), but the method is called not from the thread that starts the rpc call but from an executor. I pasted stack trace for the two relevant threads here. I am not an expert on this, but it looks like

grpc-java/core/src/main/java/io/grpc/internal/ClientCallImpl.java

Line 198 in 4abb022

callExecutor.execute(new ContextRunnable(context) {

might have something to do with it.

SEVERE: Exception while executing runnable io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$3@795fea11
io.grpc.StatusRuntimeException: INTERNAL: More than one value received for unary call
at io.grpc.Status.asRuntimeException(Status.java:430)
at io.grpc.stub.ClientCalls$UnaryStreamToFuture.onMessage(ClientCalls.java:290)
at io.gapi.gax.grpc.PageStreamingCallable$PageStreamingCall.onMessage(PageStreamingCallable.java:161)
at io.gapi.gax.grpc.CompoundClientCall$InnerListener.onMessage(CompoundClientCall.java:70)
at io.gapi.gax.grpc.RetryingCallable$RetryingCall.onClose(RetryingCallable.java:166)
at io.gapi.gax.grpc.CompoundClientCall$InnerListener.onClose(CompoundClientCall.java:75)
at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$3.run(ClientCallImpl.java:320)
at io.grpc.internal.SerializingExecutor$TaskRunner.run(SerializingExecutor.java:154)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)

sun.misc.Unsafe.park(Native Method)
java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
java.util.concurrent.locks.AbstractQueuedSynchronizer.parkAndCheckInterrupt(AbstractQueuedSynchronizer.java:836)
java.util.concurrent.locks.AbstractQueuedSynchronizer.doAcquireSharedInterruptibly(AbstractQueuedSynchronizer.java:997)
java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1304)
com.google.common.util.concurrent.AbstractFuture$Sync.get(AbstractFuture.java:285)
com.google.common.util.concurrent.AbstractFuture.get(AbstractFuture.java:116)
io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:151)
io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:106)
io.gapi.gax.grpc.ApiCallable.call(ApiCallable.java:140)
io.gapi.gax.grpc.ApiCallable.call(ApiCallable.java:148)
com.google.gapi.examples.PubsubClientExample.execute(PubsubClientExample.java:104)
com.google.gapi.examples.PubsubClientExample.main(PubsubClientExample.java:29)

[carl-mastrangelo]

This still appears to be correct behavior. Perhaps I am still not understanding you? An exception in the application code should not prevent graceful teardown of the RPC. From looking at your stack trace, it looks like you are calling the wrong method, or there is a server bug that is responding incorrectly to your client. In either case, the Client Call will still want to gracefully tear down the RPC. Because it is an async API under the hood, Callbacks are invoked when an event happens. If there is an exception in one of the application provided callbacks, there isn't any way back to the application code to propagate it to. The only way we can get it back to your code is through the listener.onClose()

Could you provide a brief snippet of calls you make, and how you expect the exception to propagate? Or perhaps a link to the code where the error happened?

[pongad]

@carl-mastrangelo , I think I understand you now. Am I right to say that, at least when using ClientCallImpl, throwing from any methods of Listener will propagate to onClose, but onClose itself probably shouldn't throw? If that's the rule, I can find a way to propagate the error back to the calling thread in the application layer.

[carl-mastrangelo]

Looking more closely, It doesn't appear that we actually propagate the specific exception, though we could change it to do so. The reason for this is because if there is an exception in a listener, there isn't any way to tell the server about it. The spec doesn't allow trailing headers from client responses. Also, typically onClose is called with the response from the server, rather than the exception. So, the flow of events looks like this:

1. Application throws in CL.onHeaders.
2. Exception is caught in CL.headersRead
3. cancel() is called
4. stream.cancel() is called
5. ...
6. Server responds with headers
7. CCI.L closed() is called
8. Server headers are sent to onClose()

It looks like you are trying to write framework code, so I believe the correct course of action for you is to wrap your callbacks in a try-finally. We don't tell you that there was an exception, but we do clean up and tell you the server response to the exception.

For regular calls on ClientCall, those can throw if the arguments are wrong, because they are only safe to call from the application code.

It should be safe for onClose to throw, but again, we still lack a way of propagating it back to you. I think your best bet is to try-finally wrap your callbacks for the time being.

[carl-mastrangelo]

cc: @ejona86 since this affects stubbed calls too.

[pongad]

Makes sense. I'll close this issue. Thank you very much for you help @carl-mastrangelo !

[ejona86]

We've had some talk about propagating an exception for Context-caused cancellation, and that could be re-used for this case as well. In general, we aren't wanting applications to throw in the callbacks. They can and it will be handled, but you will get the log-spam.

I'm not super-concerned, because it does seem like we could add the cause to the Status in the future if it turns out to be a good idea.