Integrate CoroutineContext and Scope with generated server base

[rouzwawi]

This should also remedy #3 by allowing a CoroutineExceptionHandler to be added to the CoroutineContext, like:

val handler = CoroutineExceptionHandler { _, exception ->
    println("Caught $exception")
}

class GreeterImpl : GreeterGrpcKt.GreeterImplBase(
    coroutineContext = Dispatchers.Default + handler
) {
    override fun greet(request: GreetRequest) = async<GreetReply> {
        throw SomeApplicationSpecificException("oops!")
    }
}

[wfhartford]

This looks like a promising solution, and seems like more or less the right way to handle exceptions with coroutines. There are however two concerns I have with the current solution:

• With the default behaviour (no parameter passed to the ...ImplBase constructor), exceptions thrown by the service code are not logged in any way. This can obviously be addressed by adding a CoroutineExceptionHandler, but in my opinion, the default behaviour should show the developer the exception in some way.
• Exceptions thrown by service code do not find their way to a ServerInterceptor configured on the ServerBuilder.

The code generated for Java accomplishes both of these by allowing an exception to propagate out of the MethodHnadlers.invoke() methods.

What are your thoughts?

[rouzwawi]

I've tried the new setup with a server that throws simple a RuntimeException on the calls and I've seen the threads print out the exception:

class GreeterImpl : GreeterGrpcKt.GreeterImplBase(
    // tried with and without this
    coroutineContext = newFixedThreadPoolContext(4, "server-pool")
) {

    override fun greet(request: GreetRequest) = async<GreetReply> {
        throw RuntimeException("bork")
    }
}

I've seen both:

Exception in thread "DefaultDispatcher-worker-3" java.lang.RuntimeException: bork
	at io.rouz.greeter.GreeterImpl$greet$1.doResume(GreeterImpl.kt:42)
	at kotlin.coroutines.experimental.jvm.internal.CoroutineImpl.resume(CoroutineImpl.kt:42)
	at kotlinx.coroutines.experimental.DispatchedTask$DefaultImpls.run(Dispatched.kt:168)
	at kotlinx.coroutines.experimental.DispatchedContinuation.run(Dispatched.kt:13)
	at kotlinx.coroutines.experimental.scheduling.Task.run(Tasks.kt:94)
	at kotlinx.coroutines.experimental.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:586)
	at kotlinx.coroutines.experimental.scheduling.CoroutineScheduler.access$runSafely(CoroutineScheduler.kt:60)
	at kotlinx.coroutines.experimental.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:732)

Exception in thread "server-pool-1" java.lang.RuntimeException: bork
	at io.rouz.greeter.GreeterImpl$greet$1.doResume(GreeterImpl.kt:42)
	at kotlin.coroutines.experimental.jvm.internal.CoroutineImpl.resume(CoroutineImpl.kt:42)
	at kotlinx.coroutines.experimental.DispatchedTask$DefaultImpls.run(Dispatched.kt:168)
	at kotlinx.coroutines.experimental.DispatchedContinuation.run(Dispatched.kt:13)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Note that initially I did not see these. But that was because I had both the server and the client created in the same main method. This caused the main method and the JVM to exit as soon as the client received the failure, which lead to that the server thread stack trace was not printed. This was not an issue when the server was long lived, and I could see the exceptions printed consistently.

On the second note, I'm not so sure if handling these kinds of exceptions in a ServerInterceptor is such a good idea as it can easily miss exceptions being thrown anywhere except for in the immediate invocation of the server methods. See the following example:

public void greet(GreetRequest request, StreamObserver<GreetReply> responseObserver) {
  // simulate asynchronous execution
  executor.execute(() -> {
    throw new RuntimeException("bork");
  });
}

In this situation, the exception is never seen by the ServerInterceptor chain and just leaves the request hanging. In the generated Kotlin base server, I've tried to account for this in the tryCatchingStatus function which will complete the request when it sees any exception from the user code.

I don't see any good way to get the exceptions to propagate to the ServerInterceptor other than blocking in the request handler or the returned StreamObserver implementations.

If the first point is solved, how important is the second point for you? I think it's a matter of whether the gRPC threads or the coroutine context is executing the user code and thus where this kind of exception handling should reside.

[wfhartford]

I've confirmed that you're right on the first point. I must have been encountering the same issue as you where the server was exiting before the exception finished propagating.

The interceptor behaviour is really not important for me. I haven't seen it in any official documentation, only in a Stack Overflow answer and a GitHub issue discussion.

There are two arguments I can think of in favour of trying propagating the exception:

• Handling the exception in the interceptor provides a nice way to apply some fallback exception handling across an entire server, rather than on a per-service basis as the CoroutineExceptionHandler does.
• It would probably be expected by someone writing a Kotlin service to add to an existing Java service infrastructure. I seems like a reasonable assumption that an interceptor that gets exceptions from all my Java services would also get exceptions from Kotlin services running in the same server, or another similarly configured server.

[rouzwawi]

@wfhartford I'll create a separate issue for trying to get exceptions to bubble up to the interceptor. At the moment, I'm not sure how that can be achieved without artificially blocking the server thread. Let me know if you have any ideas.

[wfhartford]

Sounds good @rouzwawi, I think you're correct that the main server thread would have to be blocked, and it would be nice to avoid that.