Call cancellation. #7241
Call cancellation. #7241
Conversation
|
Alternatives: Cancellation token on call? Like a ListenableFuture (doesn't work for Android < 24) Or passed into chain.proceed? |
|
This form is safer than supporting some blocking operation, since a call may never actually get executed or cancelled. |
| * Most cancellation should be handled via existing | ||
| * [execute], [enqueue] and executeAsync behaviour. | ||
| */ | ||
| actual fun onCancel(cancelFn: () -> Unit) |
There was a problem hiding this comment.
should this be Runnable for our Java language friends?
There was a problem hiding this comment.
It's in common, I could typealias it, but maybe a single use "fun interface" is better?
There was a problem hiding this comment.
I think adding a supporting class here is overkill, anyone using this from java can return Unit.Instance, or null. Or you feel it's important to be a pleasant API?
There was a problem hiding this comment.
Alternately, would it make sense to let callers provide an entire EventListener here? That gives us and callers lots of power and keeps our API compact?
There was a problem hiding this comment.
Yep - let's give it a go!
There was a problem hiding this comment.
It will make this cancellation callback JVM only. EventListener isn't multiplatform friendly (proxy, inetaddress, HttpUrl).
I'll try this and see how it goes.
We can some KMP later, as we get more idea of what it needs to be.
There was a problem hiding this comment.
we can have something like chain.withEventListener() and that can register an additional listener in RealCall.eventListener, which is essentially the event listener holder.
But it becomes complicated. what happens if the call was cancelled just as your interceptor is run. do we special case cancel and call end to always be called?
okhttp/src/jvmMain/kotlin/okhttp3/internal/connection/RealCall.kt
Outdated
Show resolved
Hide resolved
okhttp/src/jvmMain/kotlin/okhttp3/internal/connection/RealCall.kt
Outdated
Show resolved
Hide resolved
|
I won't try to rush this in, but I'll create some tests with the current code so as we discuss the right options we can see how it affects those. |
|
We could also support it now but minimise the blast radius, by putting it on |
|
@swankjesse what do you think about minimising the blast radius of the change, a new method on chain, would be usable for Interceptors, without a new public API for call cancellations for all clients. Most clients can use the Callback.onFailure to check for cancellation. |
#fixes #7164