You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2010/01/08 20:17:49 UTC
svn commit: r897305 - in /httpcomponents/httpclient/trunk: RELEASE_NOTES.txt
httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java
Author: olegk
Date: Fri Jan 8 19:17:48 2010
New Revision: 897305
URL: http://svn.apache.org/viewvc?rev=897305&view=rev
Log:
HTTPCLIENT-902: HttpRequestRetryHandler not called on I/O exceptions thrown when opening a new connection
Added:
httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java (with props)
Modified:
httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
Modified: httpcomponents/httpclient/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/RELEASE_NOTES.txt?rev=897305&r1=897304&r2=897305&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpclient/trunk/RELEASE_NOTES.txt Fri Jan 8 19:17:48 2010
@@ -1,3 +1,11 @@
+Changes since 4.1 ALPHA1
+-------------------
+
+* [HTTPCLIENT-902] HttpRequestRetryHandler not called on I/O exceptions
+ thrown when opening a new connection.
+ Contributed by Olivier Lamy <olamy at apache.org> and
+ Oleg Kalnichevski <olegk at apache.org>
+
Release 4.1 ALPHA1
-------------------
Modified: httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java?rev=897305&r1=897304&r2=897305&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java (original)
+++ httpcomponents/httpclient/trunk/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java Fri Jan 8 19:17:48 2010
@@ -183,6 +183,8 @@
protected final AuthState targetAuthState;
protected final AuthState proxyAuthState;
+
+ private int execCount;
private int redirectCount;
@@ -297,6 +299,7 @@
this.managedConn = null;
+ this.execCount = 0;
this.redirectCount = 0;
this.maxRedirects = this.params.getIntParameter(ClientPNames.MAX_REDIRECTS, 100);
this.targetAuthState = new AuthState();
@@ -361,8 +364,6 @@
long timeout = ConnManagerParams.getTimeout(params);
- int execCount = 0;
-
boolean reuse = false;
boolean done = false;
try {
@@ -412,15 +413,8 @@
((AbortableHttpRequest) orig).setReleaseTrigger(managedConn);
}
- // Reopen connection if needed
- if (!managedConn.isOpen()) {
- managedConn.open(route, context, params);
- } else {
- managedConn.setSocketTimeout(HttpConnectionParams.getSoTimeout(params));
- }
-
try {
- establishRoute(route, context);
+ tryConnect(roureq, context);
} catch (TunnelRefusedException ex) {
if (this.log.isDebugEnabled()) {
this.log.debug(ex.getMessage());
@@ -459,68 +453,7 @@
// Run request protocol interceptors
requestExec.preProcess(wrapper, httpProcessor, context);
- boolean retrying = true;
- Exception retryReason = null;
- while (retrying) {
- // Increment total exec count (with redirects)
- execCount++;
- // Increment exec count for this particular request
- wrapper.incrementExecCount();
- if (wrapper.getExecCount() > 1 && !wrapper.isRepeatable()) {
- this.log.debug("Cannot retry non-repeatable request");
- if (retryReason != null) {
- throw new NonRepeatableRequestException("Cannot retry request " +
- "with a non-repeatable request entity. The cause lists the " +
- "reason the original request failed.", retryReason);
- } else {
- throw new NonRepeatableRequestException("Cannot retry request " +
- "with a non-repeatable request entity.");
- }
- }
-
- try {
- if (this.log.isDebugEnabled()) {
- this.log.debug("Attempt " + execCount + " to execute request");
- }
- response = requestExec.execute(wrapper, managedConn, context);
- retrying = false;
-
- } catch (IOException ex) {
- this.log.debug("Closing the connection.");
- try {
- managedConn.close();
- } catch (IOException ignore) {
- }
- if (retryHandler.retryRequest(ex, execCount, context)) {
- if (this.log.isInfoEnabled()) {
- this.log.info("I/O exception ("+ ex.getClass().getName() +
- ") caught when processing request: "
- + ex.getMessage());
- }
- if (this.log.isDebugEnabled()) {
- this.log.debug(ex.getMessage(), ex);
- }
- this.log.info("Retrying request");
- retryReason = ex;
- } else {
- throw ex;
- }
-
- // If we have a direct route to the target host
- // just re-open connection and re-try the request
- if (!route.isTunnelled()) {
- this.log.debug("Reopening the direct connection.");
- managedConn.open(route, context, params);
- } else {
- // otherwise give up
- this.log.debug("Proxied connection. Need to start over.");
- retrying = false;
- }
-
- }
-
- }
-
+ response = tryExecute(roureq, context);
if (response == null) {
// Need to start over
continue;
@@ -615,6 +548,120 @@
} // execute
/**
+ * Establish connection either directly or through a tunnel and retry in case of
+ * a recoverable I/O failure
+ */
+ private void tryConnect(
+ final RoutedRequest req, final HttpContext context) throws HttpException, IOException {
+ HttpRoute route = req.getRoute();
+
+ boolean retrying = true;
+ while (retrying) {
+ // Increment total exec count
+ execCount++;
+ try {
+ if (!managedConn.isOpen()) {
+ managedConn.open(route, context, params);
+ } else {
+ managedConn.setSocketTimeout(HttpConnectionParams.getSoTimeout(params));
+ }
+ establishRoute(route, context);
+ retrying = false;
+ } catch (IOException ex) {
+ try {
+ managedConn.close();
+ } catch (IOException ignore) {
+ }
+ if (retryHandler.retryRequest(ex, execCount, context)) {
+ if (this.log.isInfoEnabled()) {
+ this.log.info("I/O exception ("+ ex.getClass().getName() +
+ ") caught when connecting to the target host: "
+ + ex.getMessage());
+ }
+ if (this.log.isDebugEnabled()) {
+ this.log.debug(ex.getMessage(), ex);
+ }
+ this.log.info("Retrying connect");
+ } else {
+ throw ex;
+ }
+ }
+ }
+ }
+
+ /**
+ * Execute request and retry in case of a recoverable I/O failure
+ */
+ private HttpResponse tryExecute(
+ final RoutedRequest req, final HttpContext context) throws HttpException, IOException {
+ RequestWrapper wrapper = req.getRequest();
+ HttpRoute route = req.getRoute();
+ HttpResponse response = null;
+
+ boolean retrying = true;
+ Exception retryReason = null;
+ while (retrying) {
+ // Increment total exec count (with redirects)
+ execCount++;
+ // Increment exec count for this particular request
+ wrapper.incrementExecCount();
+ if (wrapper.getExecCount() > 1 && !wrapper.isRepeatable()) {
+ this.log.debug("Cannot retry non-repeatable request");
+ if (retryReason != null) {
+ throw new NonRepeatableRequestException("Cannot retry request " +
+ "with a non-repeatable request entity. The cause lists the " +
+ "reason the original request failed.", retryReason);
+ } else {
+ throw new NonRepeatableRequestException("Cannot retry request " +
+ "with a non-repeatable request entity.");
+ }
+ }
+
+ try {
+ if (this.log.isDebugEnabled()) {
+ this.log.debug("Attempt " + execCount + " to execute request");
+ }
+ response = requestExec.execute(wrapper, managedConn, context);
+ retrying = false;
+
+ } catch (IOException ex) {
+ this.log.debug("Closing the connection.");
+ try {
+ managedConn.close();
+ } catch (IOException ignore) {
+ }
+ if (retryHandler.retryRequest(ex, execCount, context)) {
+ if (this.log.isInfoEnabled()) {
+ this.log.info("I/O exception ("+ ex.getClass().getName() +
+ ") caught when processing request: "
+ + ex.getMessage());
+ }
+ if (this.log.isDebugEnabled()) {
+ this.log.debug(ex.getMessage(), ex);
+ }
+ this.log.info("Retrying request");
+ retryReason = ex;
+ } else {
+ throw ex;
+ }
+
+ // If we have a direct route to the target host
+ // just re-open connection and re-try the request
+ if (!route.isTunnelled()) {
+ this.log.debug("Reopening the direct connection.");
+ managedConn.open(route, context, params);
+ } else {
+ // otherwise give up
+ this.log.debug("Proxied connection. Need to start over.");
+ retrying = false;
+ }
+
+ }
+ }
+ return response;
+ }
+
+ /**
* Returns the connection back to the connection manager
* and prepares for retrieving a new connection during
* the next request.
Added: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java?rev=897305&view=auto
==============================================================================
--- httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java (added)
+++ httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java Fri Jan 8 19:17:48 2010
@@ -0,0 +1,98 @@
+/*
+ * ====================================================================
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation. For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+package org.apache.http.impl.client;
+
+import java.io.IOException;
+import java.net.UnknownHostException;
+
+import junit.framework.TestCase;
+
+import org.apache.http.client.HttpRequestRetryHandler;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.scheme.PlainSocketFactory;
+import org.apache.http.conn.scheme.Scheme;
+import org.apache.http.conn.scheme.SchemeRegistry;
+import org.apache.http.impl.conn.SingleClientConnManager;
+import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
+import org.apache.http.params.HttpConnectionParams;
+import org.apache.http.protocol.HttpContext;
+
+public class TestRequestRetryHandler extends TestCase {
+
+ public void testUseRetryHandlerInConnectionTimeOutWithThreadSafeClientConnManager()
+ throws Exception {
+
+ SchemeRegistry schemeRegistry = new SchemeRegistry();
+ schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
+ ClientConnectionManager connManager = new ThreadSafeClientConnManager(schemeRegistry);
+
+ assertOnRetry(connManager);
+ }
+
+ public void testUseRetryHandlerInConnectionTimeOutWithSingleClientConnManager()
+ throws Exception {
+
+ SchemeRegistry schemeRegistry = new SchemeRegistry();
+ schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
+ ClientConnectionManager connManager = new SingleClientConnManager(schemeRegistry);
+ assertOnRetry(connManager);
+ }
+
+ protected void assertOnRetry(ClientConnectionManager connManager) throws Exception {
+ SchemeRegistry schemeRegistry = new SchemeRegistry();
+ schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));
+
+ DefaultHttpClient client = new DefaultHttpClient(connManager);
+ TestHttpRequestRetryHandler testRetryHandler = new TestHttpRequestRetryHandler();
+ client.setHttpRequestRetryHandler(testRetryHandler);
+
+ HttpRequestBase request = new HttpGet("http://www.complete.garbage");
+
+ HttpConnectionParams.setConnectionTimeout(request.getParams(), 1);
+ try {
+ client.execute(request);
+ } catch (UnknownHostException ex) {
+ assertEquals(2, testRetryHandler.retryNumber);
+ }
+ }
+
+ static class TestHttpRequestRetryHandler implements HttpRequestRetryHandler {
+
+ int retryNumber = 0;
+
+ public boolean retryRequest(IOException exception, int executionCount, HttpContext context) {
+ retryNumber++;
+ if (executionCount < 2) {
+ return true;
+ }
+ return false;
+ }
+
+ }
+
+}
Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: httpcomponents/httpclient/trunk/httpclient/src/test/java/org/apache/http/impl/client/TestRequestRetryHandler.java
------------------------------------------------------------------------------
svn:keywords = Rev Date