Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a canceled event to EventListener #5790

Merged
merged 1 commit into from
Feb 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ class LoggingEventListener private constructor(
logWithTime("callFailed: $ioe")
}

override fun canceled(call: Call) {
logWithTime("canceled")
}

private fun logWithTime(message: String) {
val timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNs)
logger.log("[$timeMs ms] $message")
Expand Down
5 changes: 5 additions & 0 deletions okhttp-testing-support/src/main/java/okhttp3/CallEvent.kt
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ sealed class CallEvent {
val ioe: IOException
) : CallEvent()

data class Canceled(
override val timestampNs: Long,
override val call: Call
) : CallEvent()

data class RequestHeadersStart(
override val timestampNs: Long,
override val call: Call
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,12 @@ class ClientRuleEventListener(val delegate: EventListener = NONE, var logger: (S
delegate.callFailed(call, ioe)
}

override fun canceled(call: Call) {
logWithTime("canceled")

delegate.canceled(call)
}

private fun logWithTime(message: String) {
val timeMs = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNs)
logger.invoke("[$timeMs ms] $message")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit
import okhttp3.CallEvent.CallEnd
import okhttp3.CallEvent.CallFailed
import okhttp3.CallEvent.CallStart
import okhttp3.CallEvent.Canceled
import okhttp3.CallEvent.ConnectEnd
import okhttp3.CallEvent.ConnectFailed
import okhttp3.CallEvent.ConnectStart
Expand Down Expand Up @@ -249,4 +250,8 @@ open class RecordingEventListener : EventListener() {
call: Call,
ioe: IOException
) = logEvent(CallFailed(System.nanoTime(), call, ioe))

override fun canceled(
call: Call
) = logEvent(Canceled(System.nanoTime(), call))
}
21 changes: 21 additions & 0 deletions okhttp/src/main/java/okhttp3/EventListener.kt
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,27 @@ abstract class EventListener {
) {
}

/**
* Invoked when a call is canceled.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice docs!

*
* Like all methods in this interface, this is invoked on the thread that triggered the event. But
* while other events occur sequentially; cancels may occur concurrently with other events. For
* example, thread A may be executing [responseBodyStart] while thread B executes [canceled].
* Implementations must support such concurrent calls.
*
* Note that cancellation is best-effort and that a call may proceed normally after it has been
* canceled. For example, happy-path events like [requestHeadersStart] and [requestHeadersEnd] may
* occur after a call is canceled. Typically cancellation takes effect when an expensive I/O
* operation is required.
*
* This is invoked at most once, even if [Call.cancel] is invoked multiple times. It may be
* invoked at any point in a call's life, including before [callStart] and after [callEnd].
*/
open fun canceled(
call: Call
) {
}

interface Factory {
/**
* Creates an instance of the [EventListener] for a particular [Call]. The returned
Expand Down
2 changes: 2 additions & 0 deletions okhttp/src/main/java/okhttp3/internal/connection/RealCall.kt
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,13 @@ class RealCall(
val exchangeToCancel: Exchange?
val connectionToCancel: RealConnection?
synchronized(connectionPool) {
if (canceled) return // Already canceled.
canceled = true
exchangeToCancel = exchange
connectionToCancel = exchangeFinder?.connectingConnection() ?: connection
}
exchangeToCancel?.cancel() ?: connectionToCancel?.cancel()
eventListener.canceled(this)
}

override fun isCanceled(): Boolean {
Expand Down
36 changes: 35 additions & 1 deletion okhttp/src/test/java/okhttp3/EventListenerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,41 @@ public final class EventListenerTest {
assertThat(expected.getMessage()).isEqualTo("Canceled");
}

assertThat(listener.recordedEventTypes()).containsExactly("CallStart", "CallFailed");
assertThat(listener.recordedEventTypes()).containsExactly(
"Canceled", "CallStart", "CallFailed");
}

@Test public void cancelAsyncCall() throws IOException {
server.enqueue(new MockResponse()
.setBody("abc"));

Call call = client.newCall(new Request.Builder()
.url(server.url("/"))
.build());
call.enqueue(new Callback() {
@Override public void onFailure(Call call, IOException e) {
}

@Override public void onResponse(Call call, Response response) throws IOException {
response.close();
}
});
call.cancel();

assertThat(listener.recordedEventTypes()).contains("Canceled");
}

@Test public void multipleCancelsEmitsOnlyOneEvent() throws IOException {
server.enqueue(new MockResponse()
.setBody("abc"));

Call call = client.newCall(new Request.Builder()
.url(server.url("/"))
.build());
call.cancel();
call.cancel();

assertThat(listener.recordedEventTypes()).containsExactly("Canceled");
}

private void assertSuccessfulEventOrder(Matcher<Response> responseMatcher) throws IOException {
Expand Down
1 change: 1 addition & 0 deletions okhttp/src/test/java/okhttp3/KotlinSourceModernTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ class KotlinSourceModernTest {
override fun responseFailed(call: Call, ioe: IOException) = TODO()
override fun callEnd(call: Call) = TODO()
override fun callFailed(call: Call, ioe: IOException) = TODO()
override fun canceled(call: Call) = TODO()
}
val none: EventListener = EventListener.NONE
}
Expand Down
4 changes: 4 additions & 0 deletions samples/guide/src/main/java/okhttp3/recipes/PrintEvents.java
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,9 @@ private void printEvent(String name) {
@Override public void callFailed(Call call, IOException ioe) {
printEvent("callFailed");
}

@Override public void canceled(Call call) {
printEvent("canceled");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,9 @@ private void printEvent(String name) {
@Override public void callFailed(Call call, IOException ioe) {
printEvent("callFailed");
}

@Override public void canceled(Call call) {
printEvent("canceled");
}
}
}