From aac6c8da0720c53ccd8aa8385f8f250f78a7b61c Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Sat, 23 Feb 2019 18:38:47 -0500 Subject: [PATCH] Introduce EventListener.requestFailed, responseFailed events These replace requestBodyEnd() / responseBodyEnd() in some failure scenarios. They may also be issued in cases where no event was published previously. --- .../okhttp3/logging/LoggingEventListener.java | 10 +++++++ .../src/test/java/okhttp3/CallTest.java | 20 ++++++++++++- .../src/test/java/okhttp3/DuplexTest.java | 2 +- .../test/java/okhttp3/EventListenerTest.java | 12 ++++---- .../java/okhttp3/RecordingEventListener.java | 30 +++++++++++++++++-- .../src/main/java/okhttp3/EventListener.java | 20 ++++++++++++- .../okhttp3/internal/connection/Exchange.java | 17 +++++++++-- .../java/okhttp3/recipes/PrintEvents.java | 8 +++++ .../recipes/PrintEventsNonConcurrent.java | 8 +++++ 9 files changed, 115 insertions(+), 12 deletions(-) diff --git a/okhttp-logging-interceptor/src/main/java/okhttp3/logging/LoggingEventListener.java b/okhttp-logging-interceptor/src/main/java/okhttp3/logging/LoggingEventListener.java index da38261bf..b0136d124 100644 --- a/okhttp-logging-interceptor/src/main/java/okhttp3/logging/LoggingEventListener.java +++ b/okhttp-logging-interceptor/src/main/java/okhttp3/logging/LoggingEventListener.java @@ -124,6 +124,11 @@ public final class LoggingEventListener extends EventListener { logWithTime("requestBodyEnd: byteCount=" + byteCount); } + @Override + public void requestFailed(Call call, IOException ioe) { + logWithTime("requestFailed: " + ioe); + } + @Override public void responseHeadersStart(Call call) { logWithTime("responseHeadersStart"); @@ -144,6 +149,11 @@ public final class LoggingEventListener extends EventListener { logWithTime("responseBodyEnd: byteCount=" + byteCount); } + @Override + public void responseFailed(Call call, IOException ioe) { + logWithTime("responseFailed: " + ioe); + } + @Override public void callEnd(Call call) { logWithTime("callEnd"); diff --git a/okhttp-tests/src/test/java/okhttp3/CallTest.java b/okhttp-tests/src/test/java/okhttp3/CallTest.java index 66e5ba348..66151c5d9 100644 --- a/okhttp-tests/src/test/java/okhttp3/CallTest.java +++ b/okhttp-tests/src/test/java/okhttp3/CallTest.java @@ -56,6 +56,10 @@ import javax.net.ssl.SSLPeerUnverifiedException; import javax.net.ssl.SSLProtocolException; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; +import okhttp3.RecordingEventListener.CallEnd; +import okhttp3.RecordingEventListener.ConnectionAcquired; +import okhttp3.RecordingEventListener.ConnectionReleased; +import okhttp3.RecordingEventListener.ResponseFailed; import okhttp3.internal.DoubleInetAddressDns; import okhttp3.internal.RecordingOkAuthenticator; import okhttp3.internal.Util; @@ -104,8 +108,11 @@ public final class CallTest { @Rule public final MockWebServer server2 = new MockWebServer(); @Rule public final InMemoryFileSystem fileSystem = new InMemoryFileSystem(); + private final RecordingEventListener listener = new RecordingEventListener(); private HandshakeCertificates handshakeCertificates = localhost(); - private OkHttpClient client = defaultClient(); + private OkHttpClient client = defaultClient().newBuilder() + .eventListener(listener) + .build(); private RecordingCallback callback = new RecordingCallback(); private TestLogHandler logHandler = new TestLogHandler(); private Cache cache = new Cache(new File("/cache/"), Integer.MAX_VALUE, fileSystem); @@ -1041,6 +1048,17 @@ public final class CallTest { executeSynchronously("/").assertBody("seed connection pool"); executeSynchronously("/").assertBody("retry success"); + + // The call that seeds the connection pool. + listener.removeUpToEvent(CallEnd.class); + + // The ResponseFailed event is not necessarily fatal! + listener.removeUpToEvent(ConnectionAcquired.class); + listener.removeUpToEvent(ResponseFailed.class); + listener.removeUpToEvent(ConnectionReleased.class); + listener.removeUpToEvent(ConnectionAcquired.class); + listener.removeUpToEvent(ConnectionReleased.class); + listener.removeUpToEvent(CallEnd.class); } @Test public void recoverWhenRetryOnConnectionFailureIsTrue_HTTP2() throws Exception { diff --git a/okhttp-tests/src/test/java/okhttp3/DuplexTest.java b/okhttp-tests/src/test/java/okhttp3/DuplexTest.java index 1911ef19b..8d8433431 100644 --- a/okhttp-tests/src/test/java/okhttp3/DuplexTest.java +++ b/okhttp-tests/src/test/java/okhttp3/DuplexTest.java @@ -319,7 +319,7 @@ public final class DuplexTest { "RequestHeadersStart", "RequestHeadersEnd", "RequestBodyStart", "ResponseHeadersStart", "ResponseHeadersEnd", "ResponseBodyStart", "ResponseBodyEnd", "RequestHeadersStart", "RequestHeadersEnd", "ResponseHeadersStart", "ResponseHeadersEnd", "ResponseBodyStart", - "ResponseBodyEnd", "ConnectionReleased", "CallEnd", "RequestBodyEnd"); + "ResponseBodyEnd", "ConnectionReleased", "CallEnd", "RequestFailed"); assertEquals(expectedEvents, listener.recordedEventTypes()); } diff --git a/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java b/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java index 7c2a4d49f..1bf37a086 100644 --- a/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java +++ b/okhttp-tests/src/test/java/okhttp3/EventListenerTest.java @@ -38,6 +38,7 @@ import okhttp3.RecordingEventListener.DnsStart; import okhttp3.RecordingEventListener.RequestBodyEnd; import okhttp3.RecordingEventListener.RequestHeadersEnd; import okhttp3.RecordingEventListener.ResponseBodyEnd; +import okhttp3.RecordingEventListener.ResponseFailed; import okhttp3.RecordingEventListener.ResponseHeadersEnd; import okhttp3.RecordingEventListener.SecureConnectEnd; import okhttp3.RecordingEventListener.SecureConnectStart; @@ -168,7 +169,8 @@ public final class EventListenerTest { List expectedEvents = Arrays.asList("CallStart", "DnsStart", "DnsEnd", "ConnectStart", "ConnectEnd", "ConnectionAcquired", "RequestHeadersStart", - "RequestHeadersEnd", "ResponseHeadersStart", "ConnectionReleased", "CallFailed"); + "RequestHeadersEnd", "ResponseHeadersStart", "ResponseFailed", "ConnectionReleased", + "CallFailed"); assertEquals(expectedEvents, listener.recordedEventTypes()); } @@ -197,10 +199,10 @@ public final class EventListenerTest { List expectedEvents = Arrays.asList("CallStart", "DnsStart", "DnsEnd", "ConnectStart", "ConnectEnd", "ConnectionAcquired", "RequestHeadersStart", "RequestHeadersEnd", "ResponseHeadersStart", "ResponseHeadersEnd", "ResponseBodyStart", - "ResponseBodyEnd", "ConnectionReleased", "CallFailed"); + "ResponseFailed", "ConnectionReleased", "CallFailed"); assertEquals(expectedEvents, listener.recordedEventTypes()); - ResponseBodyEnd bodyEnd = listener.removeUpToEvent(ResponseBodyEnd.class); - assertEquals(5, bodyEnd.bytesRead); + ResponseFailed responseFailed = listener.removeUpToEvent(ResponseFailed.class); + assertEquals("unexpected end of stream", responseFailed.ioe.getMessage()); } @Test public void canceledCallEventSequence() { @@ -1052,7 +1054,7 @@ public final class EventListenerTest { List expectedEvents = Arrays.asList("CallStart", "DnsStart", "DnsEnd", "ConnectStart", "ConnectEnd", "ConnectionAcquired", "RequestHeadersStart", "RequestHeadersEnd", - "RequestBodyStart", "RequestBodyEnd", "ConnectionReleased", "CallFailed"); + "RequestBodyStart", "RequestFailed", "ConnectionReleased", "CallFailed"); assertEquals(expectedEvents, listener.recordedEventTypes()); } diff --git a/okhttp-tests/src/test/java/okhttp3/RecordingEventListener.java b/okhttp-tests/src/test/java/okhttp3/RecordingEventListener.java index ee26e6819..5dde53d6b 100644 --- a/okhttp-tests/src/test/java/okhttp3/RecordingEventListener.java +++ b/okhttp-tests/src/test/java/okhttp3/RecordingEventListener.java @@ -19,18 +19,18 @@ import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Proxy; -import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; import java.util.Deque; import java.util.List; +import java.util.concurrent.ConcurrentLinkedDeque; import javax.annotation.Nullable; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public final class RecordingEventListener extends EventListener { - final Deque eventSequence = new ArrayDeque<>(); + final Deque eventSequence = new ConcurrentLinkedDeque<>(); final List forbiddenLocks = new ArrayList<>(); @@ -138,6 +138,10 @@ public final class RecordingEventListener extends EventListener { logEvent(new RequestBodyEnd(call, byteCount)); } + @Override public void requestFailed(Call call, IOException ioe) { + logEvent(new RequestFailed(call, ioe)); + } + @Override public void responseHeadersStart(Call call) { logEvent(new ResponseHeadersStart(call)); } @@ -154,6 +158,10 @@ public final class RecordingEventListener extends EventListener { logEvent(new ResponseBodyEnd(call, byteCount)); } + @Override public void responseFailed(Call call, IOException ioe) { + logEvent(new ResponseFailed(call, ioe)); + } + @Override public void callEnd(Call call) { logEvent(new CallEnd(call)); } @@ -374,6 +382,15 @@ public final class RecordingEventListener extends EventListener { } } + static final class RequestFailed extends CallEvent { + final IOException ioe; + + RequestFailed(Call call, IOException ioe) { + super(call, ioe); + this.ioe = ioe; + } + } + static final class ResponseHeadersStart extends CallEvent { ResponseHeadersStart(Call call) { super(call); @@ -411,4 +428,13 @@ public final class RecordingEventListener extends EventListener { return new ResponseBodyStart(call); } } + + static final class ResponseFailed extends CallEvent { + final IOException ioe; + + ResponseFailed(Call call, IOException ioe) { + super(call, ioe); + this.ioe = ioe; + } + } } diff --git a/okhttp/src/main/java/okhttp3/EventListener.java b/okhttp/src/main/java/okhttp3/EventListener.java index bdd9f050c..28f346798 100644 --- a/okhttp/src/main/java/okhttp3/EventListener.java +++ b/okhttp/src/main/java/okhttp3/EventListener.java @@ -46,7 +46,7 @@ import javax.annotation.Nullable; * events. * *

All event methods must execute fast, without external locking, cannot throw exceptions, - * attempt to mutate the event parameters, or be reentrant back into the client. + * attempt to mutate the event parameters, or be re-entrant back into the client. * Any IO - writing to files or network should be done asynchronously. */ public abstract class EventListener { @@ -210,6 +210,15 @@ public abstract class EventListener { public void requestBodyEnd(Call call, long byteCount) { } + /** + * Invoked when a request fails to be written. + * + *

This method is invoked after {@link #requestHeadersStart} or {@link #requestBodyStart}. Note + * that request failures do not necessarily fail the entire call. + */ + public void requestFailed(Call call, IOException ioe) { + } + /** * Invoked just prior to receiving response headers. * @@ -256,6 +265,15 @@ public abstract class EventListener { public void responseBodyEnd(Call call, long byteCount) { } + /** + * Invoked when a response fails to be read. + * + *

This method is invoked after {@link #responseHeadersStart} or {@link #responseBodyStart}. + * Note that response failures do not necessarily fail the entire call. + */ + public void responseFailed(Call call, IOException ioe) { + } + /** * Invoked immediately after a call has completely ended. This includes delayed consumption * of response body by the caller. diff --git a/okhttp/src/main/java/okhttp3/internal/connection/Exchange.java b/okhttp/src/main/java/okhttp3/internal/connection/Exchange.java index aceb9843b..f340f3007 100644 --- a/okhttp/src/main/java/okhttp3/internal/connection/Exchange.java +++ b/okhttp/src/main/java/okhttp3/internal/connection/Exchange.java @@ -66,6 +66,7 @@ public final class Exchange { codec.writeRequestHeaders(request); eventListener.requestHeadersEnd(call, request); } catch (IOException e) { + eventListener.requestFailed(call, e); trackFailure(e); throw e; } @@ -82,6 +83,7 @@ public final class Exchange { try { codec.flushRequest(); } catch (IOException e) { + eventListener.requestFailed(call, e); trackFailure(e); throw e; } @@ -91,6 +93,7 @@ public final class Exchange { try { codec.finishRequest(); } catch (IOException e) { + eventListener.requestFailed(call, e); trackFailure(e); throw e; } @@ -108,6 +111,7 @@ public final class Exchange { } return result; } catch (IOException e) { + eventListener.responseFailed(call, e); trackFailure(e); throw e; } @@ -126,6 +130,7 @@ public final class Exchange { ResponseBodySource source = new ResponseBodySource(rawSource, contentLength); return new RealResponseBody(contentType, contentLength, Okio.buffer(source)); } catch (IOException e) { + eventListener.responseFailed(call, e); trackFailure(e); throw e; } @@ -167,10 +172,18 @@ public final class Exchange { trackFailure(e); } if (requestDone) { - eventListener.requestBodyEnd(call, bytesRead); + if (e != null) { + eventListener.requestFailed(call, e); + } else { + eventListener.requestBodyEnd(call, bytesRead); + } } if (responseDone) { - eventListener.responseBodyEnd(call, bytesRead); + if (e != null) { + eventListener.responseFailed(call, e); + } else { + eventListener.responseBodyEnd(call, bytesRead); + } } transmitter.exchangeMessageDone(this, requestDone, responseDone, e); } diff --git a/samples/guide/src/main/java/okhttp3/recipes/PrintEvents.java b/samples/guide/src/main/java/okhttp3/recipes/PrintEvents.java index 1366a82f1..6c9f71279 100644 --- a/samples/guide/src/main/java/okhttp3/recipes/PrintEvents.java +++ b/samples/guide/src/main/java/okhttp3/recipes/PrintEvents.java @@ -156,6 +156,10 @@ public final class PrintEvents { printEvent("requestBodyEnd"); } + @Override public void requestFailed(Call call, IOException ioe) { + printEvent("requestFailed"); + } + @Override public void responseHeadersStart(Call call) { printEvent("responseHeadersStart"); } @@ -172,6 +176,10 @@ public final class PrintEvents { printEvent("responseBodyEnd"); } + @Override public void responseFailed(Call call, IOException ioe) { + printEvent("responseFailed"); + } + @Override public void callEnd(Call call) { printEvent("callEnd"); } diff --git a/samples/guide/src/main/java/okhttp3/recipes/PrintEventsNonConcurrent.java b/samples/guide/src/main/java/okhttp3/recipes/PrintEventsNonConcurrent.java index 6acedb19e..c88895b79 100644 --- a/samples/guide/src/main/java/okhttp3/recipes/PrintEventsNonConcurrent.java +++ b/samples/guide/src/main/java/okhttp3/recipes/PrintEventsNonConcurrent.java @@ -131,6 +131,10 @@ public final class PrintEventsNonConcurrent { printEvent("requestBodyEnd"); } + @Override public void requestFailed(Call call, IOException ioe) { + printEvent("requestFailed"); + } + @Override public void responseHeadersStart(Call call) { printEvent("responseHeadersStart"); } @@ -147,6 +151,10 @@ public final class PrintEventsNonConcurrent { printEvent("responseBodyEnd"); } + @Override public void responseFailed(Call call, IOException ioe) { + printEvent("responseFailed"); + } + @Override public void callEnd(Call call) { printEvent("callEnd"); }