hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject 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
Date Fri, 08 Jan 2010 19:17:49 GMT
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



Mime
View raw message