1
0
mirror of https://github.com/square/okhttp.git synced 2025-12-03 18:31:17 +03:00

Don't retry streamed HTTP request bodies.

This commit is contained in:
jwilson
2016-06-21 23:26:37 -04:00
parent b1aae0c361
commit d8213e8156
5 changed files with 118 additions and 68 deletions

View File

@@ -29,6 +29,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLPeerUnverifiedException;
import okhttp3.AbstractResponseCache; import okhttp3.AbstractResponseCache;
import okhttp3.OkHttpClient; import okhttp3.OkHttpClient;
import okhttp3.OkUrlFactory; import okhttp3.OkUrlFactory;
@@ -83,11 +84,15 @@ public class CacheAdapterTest {
ResponseCache responseCache = new AbstractResponseCache() { ResponseCache responseCache = new AbstractResponseCache() {
@Override public CacheResponse get( @Override public CacheResponse get(
URI uri, String method, Map<String, List<String>> headers) throws IOException { URI uri, String method, Map<String, List<String>> headers) throws IOException {
try {
assertEquals(toUri(serverUrl), uri); assertEquals(toUri(serverUrl), uri);
assertEquals("GET", method); assertEquals("GET", method);
assertTrue("Arbitrary standard header not present", headers.containsKey("User-Agent")); assertTrue("Arbitrary standard header not present", headers.containsKey("User-Agent"));
assertEquals(Collections.singletonList("value1"), headers.get("key1")); assertEquals(Collections.singletonList("value1"), headers.get("key1"));
return null; return null;
} catch (Throwable t) {
throw new IOException("unexpected cache failure", t);
}
} }
}; };
setInternalCache(new CacheAdapter(responseCache)); setInternalCache(new CacheAdapter(responseCache));
@@ -105,12 +110,16 @@ public class CacheAdapterTest {
ResponseCache responseCache = new AbstractResponseCache() { ResponseCache responseCache = new AbstractResponseCache() {
@Override public CacheResponse get(URI uri, String method, Map<String, List<String>> headers) @Override public CacheResponse get(URI uri, String method, Map<String, List<String>> headers)
throws IOException { throws IOException {
try {
assertEquals("https", uri.getScheme()); assertEquals("https", uri.getScheme());
assertEquals(toUri(serverUrl), uri); assertEquals(toUri(serverUrl), uri);
assertEquals("GET", method); assertEquals("GET", method);
assertTrue("Arbitrary standard header not present", headers.containsKey("User-Agent")); assertTrue("Arbitrary standard header not present", headers.containsKey("User-Agent"));
assertEquals(Collections.singletonList("value1"), headers.get("key1")); assertEquals(Collections.singletonList("value1"), headers.get("key1"));
return null; return null;
} catch (Throwable t) {
throw new IOException("unexpected cache failure", t);
}
} }
}; };
setInternalCache(new CacheAdapter(responseCache)); setInternalCache(new CacheAdapter(responseCache));
@@ -136,6 +145,7 @@ public class CacheAdapterTest {
ResponseCache responseCache = new AbstractResponseCache() { ResponseCache responseCache = new AbstractResponseCache() {
@Override public CacheRequest put(URI uri, URLConnection connection) throws IOException { @Override public CacheRequest put(URI uri, URLConnection connection) throws IOException {
try {
assertTrue(connection instanceof HttpURLConnection); assertTrue(connection instanceof HttpURLConnection);
assertFalse(connection instanceof HttpsURLConnection); assertFalse(connection instanceof HttpsURLConnection);
@@ -157,6 +167,9 @@ public class CacheAdapterTest {
// The RI and OkHttp supports case-insensitive matching for this method. // The RI and OkHttp supports case-insensitive matching for this method.
assertEquals("c", httpUrlConnection.getHeaderField("a")); assertEquals("c", httpUrlConnection.getHeaderField("a"));
return null; return null;
} catch (Throwable t) {
throw new IOException("unexpected cache failure", t);
}
} }
}; };
setInternalCache(new CacheAdapter(responseCache)); setInternalCache(new CacheAdapter(responseCache));
@@ -175,6 +188,7 @@ public class CacheAdapterTest {
ResponseCache responseCache = new AbstractResponseCache() { ResponseCache responseCache = new AbstractResponseCache() {
@Override public CacheRequest put(URI uri, URLConnection connection) throws IOException { @Override public CacheRequest put(URI uri, URLConnection connection) throws IOException {
try {
assertTrue(connection instanceof HttpURLConnection); assertTrue(connection instanceof HttpURLConnection);
assertFalse(connection instanceof HttpsURLConnection); assertFalse(connection instanceof HttpsURLConnection);
@@ -196,6 +210,9 @@ public class CacheAdapterTest {
// The RI and OkHttp supports case-insensitive matching for this method. // The RI and OkHttp supports case-insensitive matching for this method.
assertEquals("c", httpUrlConnection.getHeaderField("a")); assertEquals("c", httpUrlConnection.getHeaderField("a"));
return null; return null;
} catch (Throwable t) {
throw new IOException("unexpected cache failure", t);
}
} }
}; };
setInternalCache(new CacheAdapter(responseCache)); setInternalCache(new CacheAdapter(responseCache));
@@ -211,6 +228,7 @@ public class CacheAdapterTest {
ResponseCache responseCache = new AbstractResponseCache() { ResponseCache responseCache = new AbstractResponseCache() {
@Override public CacheRequest put(URI uri, URLConnection connection) throws IOException { @Override public CacheRequest put(URI uri, URLConnection connection) throws IOException {
try {
assertTrue(connection instanceof HttpsURLConnection); assertTrue(connection instanceof HttpsURLConnection);
assertEquals(toUri(serverUrl), uri); assertEquals(toUri(serverUrl), uri);
assertEquals(serverUrl, connection.getURL()); assertEquals(serverUrl, connection.getURL());
@@ -229,6 +247,9 @@ public class CacheAdapterTest {
assertEquals(realHttpsUrlConnection.getLocalPrincipal(), assertEquals(realHttpsUrlConnection.getLocalPrincipal(),
cacheHttpsUrlConnection.getLocalPrincipal()); cacheHttpsUrlConnection.getLocalPrincipal());
return null; return null;
} catch (Throwable t) {
throw new IOException("unexpected cache failure", t);
}
} }
}; };
setInternalCache(new CacheAdapter(responseCache)); setInternalCache(new CacheAdapter(responseCache));

View File

@@ -322,6 +322,29 @@ public final class URLConnectionTest {
assertEquals("body", server.takeRequest().getBody().readUtf8()); assertEquals("body", server.takeRequest().getBody().readUtf8());
} }
@Test public void streamedBodyIsNotRetried() throws Exception {
server.enqueue(new MockResponse()
.setSocketPolicy(SocketPolicy.DISCONNECT_AFTER_REQUEST));
urlFactory = new OkUrlFactory(new OkHttpClient.Builder()
.dns(new DoubleInetAddressDns())
.build());
HttpURLConnection connection = urlFactory.open(server.url("/").url());
connection.setDoOutput(true);
connection.setChunkedStreamingMode(100);
OutputStream os = connection.getOutputStream();
os.write("OutputStream is no fun.".getBytes("UTF-8"));
os.close();
try {
connection.getResponseCode();
fail();
} catch (IOException expected) {
}
assertEquals(1, server.getRequestCount());
}
@Test public void getErrorStreamOnSuccessfulRequest() throws Exception { @Test public void getErrorStreamOnSuccessfulRequest() throws Exception {
server.enqueue(new MockResponse().setBody("A")); server.enqueue(new MockResponse().setBody("A"));
connection = urlFactory.open(server.url("/").url()); connection = urlFactory.open(server.url("/").url());

View File

@@ -273,6 +273,7 @@ public final class Spdy3ConnectionTest {
peer.play(); peer.play();
// play it back // play it back
final CountDownLatch maxConcurrentStreamsUpdated = new CountDownLatch(1);
final AtomicInteger maxConcurrentStreams = new AtomicInteger(); final AtomicInteger maxConcurrentStreams = new AtomicInteger();
FramedConnection.Listener listener = new FramedConnection.Listener() { FramedConnection.Listener listener = new FramedConnection.Listener() {
@Override public void onStream(FramedStream stream) throws IOException { @Override public void onStream(FramedStream stream) throws IOException {
@@ -281,6 +282,7 @@ public final class Spdy3ConnectionTest {
@Override public void onSettings(FramedConnection connection) { @Override public void onSettings(FramedConnection connection) {
maxConcurrentStreams.set(connection.maxConcurrentStreams()); maxConcurrentStreams.set(connection.maxConcurrentStreams());
maxConcurrentStreamsUpdated.countDown();
} }
}; };
FramedConnection connection = connectionBuilder(peer, SPDY3) FramedConnection connection = connectionBuilder(peer, SPDY3)
@@ -292,6 +294,7 @@ public final class Spdy3ConnectionTest {
synchronized (connection) { synchronized (connection) {
assertEquals(10, connection.peerSettings.getMaxConcurrentStreams(-1)); assertEquals(10, connection.peerSettings.getMaxConcurrentStreams(-1));
} }
maxConcurrentStreamsUpdated.await();
assertEquals(10, maxConcurrentStreams.get()); assertEquals(10, maxConcurrentStreams.get());
} }

View File

@@ -241,7 +241,7 @@ final class RealCall implements Call {
releaseConnection = false; releaseConnection = false;
} catch (RouteException e) { } catch (RouteException e) {
// The attempt to connect via a route failed. The request will not have been sent. // The attempt to connect via a route failed. The request will not have been sent.
HttpEngine retryEngine = engine.recover(e.getLastConnectException(), true, null); HttpEngine retryEngine = engine.recover(e.getLastConnectException(), true, request);
if (retryEngine != null) { if (retryEngine != null) {
releaseConnection = false; releaseConnection = false;
engine = retryEngine; engine = retryEngine;
@@ -251,7 +251,7 @@ final class RealCall implements Call {
throw e.getLastConnectException(); throw e.getLastConnectException();
} catch (IOException e) { } catch (IOException e) {
// An attempt to communicate with a server failed. The request may have been sent. // An attempt to communicate with a server failed. The request may have been sent.
HttpEngine retryEngine = engine.recover(e, false, null); HttpEngine retryEngine = engine.recover(e, false, request);
if (retryEngine != null) { if (retryEngine != null) {
releaseConnection = false; releaseConnection = false;
engine = retryEngine; engine = retryEngine;

View File

@@ -295,13 +295,17 @@ public final class HttpEngine {
* engine that should be used for the retry if {@code e} is recoverable, or null if the failure is * engine that should be used for the retry if {@code e} is recoverable, or null if the failure is
* permanent. Requests with a body can only be recovered if the body is buffered. * permanent. Requests with a body can only be recovered if the body is buffered.
*/ */
public HttpEngine recover(IOException e, boolean routeException, Response userResponse) { public HttpEngine recover(IOException e, boolean routeException, Request userRequest) {
streamAllocation.streamFailed(e); streamAllocation.streamFailed(e);
if (!client.retryOnConnectionFailure()) { if (!client.retryOnConnectionFailure()) {
return null; // The application layer has forbidden retries. return null; // The application layer has forbidden retries.
} }
if (!routeException && userRequest.body() instanceof UnrepeatableRequestBody) {
return null; // We can't send the request body again.
}
if (!isRecoverable(e, routeException)) { if (!isRecoverable(e, routeException)) {
return null; // This exception is fatal. return null; // This exception is fatal.
} }
@@ -310,11 +314,10 @@ public final class HttpEngine {
return null; // No more routes to attempt. return null; // No more routes to attempt.
} }
StreamAllocation streamAllocation = close(userResponse); StreamAllocation streamAllocation = close(null);
// For failure recovery, use the same route selector with a new connection. // For failure recovery, use the same route selector with a new connection.
return new HttpEngine(client, userRequestUrl, forWebSocket, streamAllocation, return new HttpEngine(client, userRequestUrl, forWebSocket, streamAllocation, priorResponse);
priorResponse);
} }
private boolean isRecoverable(IOException e, boolean routeException) { private boolean isRecoverable(IOException e, boolean routeException) {