okhttp
okhttp copied to clipboard
Provide a clean way for Bridging Interceptors to get a cancel signal
Application Interceptor that use an alternate transport and short circuit the interceptor chain don't have a clean way to handle cancellation.
They can either poll the call, or possibly set a specific EventListener and handle the cancel event to get this signal.
We might also want to give some way to access the EventListener (internal val on RealCall), if we'd like that to be part of the contract for these bridging interceptors.
Also wondering when implementing an Interceptor in this new world. Is this a useful base class?
abstract class CoroutineInterceptor() : Interceptor {
final override fun intercept(chain: Interceptor.Chain): Response {
return runBlocking() {
try {
interceptSuspend(chain)
} catch (ce: CancellationException) {
throw IOException(ce)
}
}
}
abstract suspend fun interceptSuspend(chain: Interceptor.Chain): Response
}
And should it cancel the runBlocking call automatically when the call is cancelled?
Not an issue if you are blocking on the call or response, but if you are doing other work around the interceptor, including another forked request (auth) then the cancellation would be helpful.
I suspect for bridging interceptors currently they end up looking like
override fun intercept(chain: Interceptor.Chain): Response {
return runBlocking {
makeRequest(engine, chain.request())
}
}
}
suspend fun makeRequest(engine: Engine, request: Request) =
suspendCancellableCoroutine<Response> { continuation ->
...
.setOnResponseTrailers { responseTrailers, streamIntel ->
if (chain.call().isCanceled()) {
continuation.cancel(CancellationException("underlying connection was cancelled"))
return@setOnResponseTrailers
}
I'm out of my depth writing a test for this, hence why it would be useful to have a solid library implementation to simplify these cases.
Definitely runBlocking is wrong here. If you want to use coroutines in the call to proceed()
I think you're in for a bad time.
Updated mine to runBlocking without IO. Not sure why runBlocking is wrong here, it achieves the same thing as the standard blocking call, uses the same thread, but allows you to do asynchronous operations elegently.
runBlocking:
* The default [CoroutineDispatcher] for this builder is an internal implementation of event loop that processes continuations
* in this blocked thread until the completion of this coroutine.
* See [CoroutineDispatcher] for the other implementations that are provided by `kotlinx.coroutines`.
This post gets into it. https://www.billjings.com/posts/title/foot-marksmanship-with-runblocking/
Coroutines are cooperative multitasking. Running a blocking call or runBlocking inside a coroutine is uncooperative and bad non-local things can happen.
I don't agree with all of that
There's another reason it won't cancel besides breaking the job chain, though, which is that runBlocking behaves just like Thread.sleep. That is, when you call it it will hard block the thread — the thread will sit there and wait for runBlocking to return.
According to docs, the thread doesn't sleep, it's a single threaded executor for the coroutines. So if the work remains non blocking, it shouldn't deadlock, and it shouldn't magically start using a bunch of extra threads. This sounds like what you want when you have a synchronous interface like Interceptor.
For your comment:
Running a blocking call or runBlocking inside a coroutine is uncooperative and bad non-local things can happen.
Agreed, but that's not what I'm proposing. A blocking call to intercept that has nothing to do with the thread until it finishes, can donate that thread to run the coroutines.
Uuugggghhhhh... I guess it breaks if the intercept method makes calls using enqueue, because they may not be processed immediately, so will waiting (noon-blockingly) for a request that can't complete until the call triggering intercept completes.