Skip to content

Split EventListener.retryDecision into retryDecision and followUpDecision #8796

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Split EventListener.retryDecision into retryDecision and followUpDecision #8796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df3504f
Split EventListener.retryDecision into retryDecision and followUpDeci…
squarejesse May 23, 2025
bfdea43
Spotless
squarejesse May 23, 2025
e2515f2
apiDump
squarejesse May 23, 2025
73d22be
Fix another test
squarejesse May 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.classpath
.kotlin
.project
.settings
.gradle
Expand Down
1 change: 1 addition & 0 deletions okhttp-sse/src/test/java/okhttp3/sse/internal/EventSourceHttpTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class EventSourceHttpTest {
"RequestHeadersEnd",
"ResponseHeadersStart",
"ResponseHeadersEnd",
"FollowUpDecision",
"ResponseBodyStart",
"ResponseBodyEnd",
"ConnectionReleased",
Expand Down
11 changes: 9 additions & 2 deletions okhttp-testing-support/src/main/kotlin/okhttp3/CallEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,14 @@ sealed class CallEvent {
data class RetryDecision(
override val timestampNs: Long,
override val call: Call,
val shouldRetry: Boolean,
val reason: String,
val exception: IOException,
val retry: Boolean,
) : CallEvent()

data class FollowUpDecision(
override val timestampNs: Long,
override val call: Call,
val networkResponse: Response,
val nextRequest: Request?,
) : CallEvent()
}
16 changes: 13 additions & 3 deletions okhttp-testing-support/src/main/kotlin/okhttp3/ClientRuleEventListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,22 @@ class ClientRuleEventListener(

override fun retryDecision(
call: Call,
shouldRetry: Boolean,
reason: String,
exception: IOException,
retry: Boolean,
) {
logWithTime("retryDecision")

delegate.retryDecision(call, shouldRetry, reason)
delegate.retryDecision(call, exception, retry)
}

override fun followUpDecision(
call: Call,
networkResponse: Response,
nextRequest: Request?,
) {
logWithTime("followUpDecision")

delegate.followUpDecision(call, networkResponse, nextRequest)
}

private fun logWithTime(message: String) {
Expand Down
16 changes: 11 additions & 5 deletions okhttp-testing-support/src/main/kotlin/okhttp3/RecordingEventListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import okhttp3.CallEvent.ConnectionAcquired
import okhttp3.CallEvent.ConnectionReleased
import okhttp3.CallEvent.DnsEnd
import okhttp3.CallEvent.DnsStart
import okhttp3.CallEvent.FollowUpDecision
import okhttp3.CallEvent.ProxySelectEnd
import okhttp3.CallEvent.ProxySelectStart
import okhttp3.CallEvent.RequestBodyEnd
Expand All @@ -53,6 +54,7 @@ import okhttp3.CallEvent.ResponseBodyStart
import okhttp3.CallEvent.ResponseFailed
import okhttp3.CallEvent.ResponseHeadersEnd
import okhttp3.CallEvent.ResponseHeadersStart
import okhttp3.CallEvent.RetryDecision
import okhttp3.CallEvent.SatisfactionFailure
import okhttp3.CallEvent.SecureConnectEnd
import okhttp3.CallEvent.SecureConnectStart
Expand Down Expand Up @@ -293,9 +295,13 @@ open class RecordingEventListener(

override fun retryDecision(
call: Call,
shouldRetry: Boolean,
reason: String,
) = logEvent(
CallEvent.RetryDecision(System.nanoTime(), call, shouldRetry, reason),
)
exception: IOException,
retry: Boolean,
) = logEvent(RetryDecision(System.nanoTime(), call, exception, retry))

override fun followUpDecision(
call: Call,
networkResponse: Response,
nextRequest: Request?,
) = logEvent(FollowUpDecision(System.nanoTime(), call, networkResponse, nextRequest))
}
3 changes: 2 additions & 1 deletion okhttp/api/android/okhttp.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ public abstract class okhttp3/EventListener {
public fun connectionReleased (Lokhttp3/Call;Lokhttp3/Connection;)V
public fun dnsEnd (Lokhttp3/Call;Ljava/lang/String;Ljava/util/List;)V
public fun dnsStart (Lokhttp3/Call;Ljava/lang/String;)V
public fun followUpDecision (Lokhttp3/Call;Lokhttp3/Response;Lokhttp3/Request;)V
public fun proxySelectEnd (Lokhttp3/Call;Lokhttp3/HttpUrl;Ljava/util/List;)V
public fun proxySelectStart (Lokhttp3/Call;Lokhttp3/HttpUrl;)V
public fun requestBodyEnd (Lokhttp3/Call;J)V
Expand All @@ -554,7 +555,7 @@ public abstract class okhttp3/EventListener {
public fun responseFailed (Lokhttp3/Call;Ljava/io/IOException;)V
public fun responseHeadersEnd (Lokhttp3/Call;Lokhttp3/Response;)V
public fun responseHeadersStart (Lokhttp3/Call;)V
public fun retryDecision (Lokhttp3/Call;ZLjava/lang/String;)V
public fun retryDecision (Lokhttp3/Call;Ljava/io/IOException;Z)V
public fun satisfactionFailure (Lokhttp3/Call;Lokhttp3/Response;)V
public fun secureConnectEnd (Lokhttp3/Call;Lokhttp3/Handshake;)V
public fun secureConnectStart (Lokhttp3/Call;)V
Expand Down
3 changes: 2 additions & 1 deletion okhttp/api/jvm/okhttp.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ public abstract class okhttp3/EventListener {
public fun connectionReleased (Lokhttp3/Call;Lokhttp3/Connection;)V
public fun dnsEnd (Lokhttp3/Call;Ljava/lang/String;Ljava/util/List;)V
public fun dnsStart (Lokhttp3/Call;Ljava/lang/String;)V
public fun followUpDecision (Lokhttp3/Call;Lokhttp3/Response;Lokhttp3/Request;)V
public fun proxySelectEnd (Lokhttp3/Call;Lokhttp3/HttpUrl;Ljava/util/List;)V
public fun proxySelectStart (Lokhttp3/Call;Lokhttp3/HttpUrl;)V
public fun requestBodyEnd (Lokhttp3/Call;J)V
Expand All @@ -554,7 +555,7 @@ public abstract class okhttp3/EventListener {
public fun responseFailed (Lokhttp3/Call;Ljava/io/IOException;)V
public fun responseHeadersEnd (Lokhttp3/Call;Lokhttp3/Response;)V
public fun responseHeadersStart (Lokhttp3/Call;)V
public fun retryDecision (Lokhttp3/Call;ZLjava/lang/String;)V
public fun retryDecision (Lokhttp3/Call;Ljava/io/IOException;Z)V
public fun satisfactionFailure (Lokhttp3/Call;Lokhttp3/Response;)V
public fun secureConnectEnd (Lokhttp3/Call;Lokhttp3/Handshake;)V
public fun secureConnectStart (Lokhttp3/Call;)V
Expand Down
53 changes: 49 additions & 4 deletions okhttp/src/commonJvmAndroid/kotlin/okhttp3/EventListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package okhttp3
import java.io.IOException
import java.net.InetAddress
import java.net.InetSocketAddress
import java.net.ProtocolException
import java.net.Proxy

/**
Expand Down Expand Up @@ -454,14 +455,58 @@ abstract class EventListener {
}

/**
* Invoked when a failed call is considered for retry.
* Invoked when OkHttp decides whether to retry after a connectivity failure.
*
* This event won't be emitted when a call isSuccessful.
* OkHttp won't retry when it is configured not to:
*
* * If retries are forbidden with [OkHttpClient.retryOnConnectionFailure]. (OkHttp's defaults
* permit retries.)
* * If OkHttp already attempted to transmit the request body, and [RequestBody.isOneShot] is
* true.
*
* It won't retry if the exception is a bug or a configuration problem, such as:
*
* * If the remote peer is untrusted: [exception] is an [SSLPeerUnverifiedException].
* * If received data is unexpected: [exception] is a [ProtocolException].
*
* Each call is made on either a reused [Connection] from a pool, or on a new connection
* established from a planned [Route]. OkHttp won't retry if it's already attempted all
* available routes.
*
* @param retry true if OkHttp will make another attempt
*/
open fun retryDecision(
call: Call,
shouldRetry: Boolean,
reason: String,
exception: IOException,
retry: Boolean,
) {
}

/**
* Invoked when OkHttp decides whether to perform a follow-up request.
*
* The network response's status code is most influential when deciding how to follow up:
*
* * For redirects (301: Moved Permanently, 302: Temporary Redirect, etc.)
* * For auth challenges (401: Unauthorized, 407: Proxy Authentication Required.)
* * For client timeouts (408: Request Time-Out.)
* * For server failures (503: Service Unavailable.)
*
* Response header values like `Location` and `Retry-After` are also considered.
*
* Client configuration may be used to make follow-up decisions, such as:
*
* * [OkHttpClient.followRedirects] must be true to follow redirects.
* * [OkHttpClient.followSslRedirects] must be true to follow redirects that add or remove HTTPS.
* * [OkHttpClient.authenticator] must respond to an authorization challenge.
*
* @param networkResponse the intermediate response that may require a follow-up request.
* @param nextRequest the follow-up request that will be made. Null if no follow-up will be made.
*/
open fun followUpDecision(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, nice. When documented properly, those are clearly two separate things.

call: Call,
networkResponse: Response,
nextRequest: Request?,
) {
}

Expand Down
4 changes: 2 additions & 2 deletions okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/RoutePlanner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ import okhttp3.HttpUrl
* a race in fast follow-up.
*
* 4. If there's no existing connection, make a list of routes (which may require blocking DNS
* lookups) and attempt a new connection them. When failures occur, retries iterate the list of
* available routes.
* lookups) and attempt new connections to them. When failures occur, retries iterate the
* list of available routes.
*
* If the pool gains an eligible connection while DNS, TCP, or TLS work is in flight, this finder
* will prefer pooled connections. Only pooled HTTP/2 connections are used for such de-duplication.
Expand Down
Loading
Loading