hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Johnson <e...@tibco.com>
Subject Re: [PATCH] Closing HttpConnection before retrying
Date Fri, 24 Jan 2003 19:59:18 GMT
Rob,

Your patch makes me slightly nervous.  Let me see if I can explain my 
reasons cogently:

    * The new "get/setReinitializeConnectionOnRetry" pair seems
      suspiciously like a property of the 'connection', rather than the
      method.  Apparently, based on your patch, you think differently.
       Can you say why?
    * If these functions are required as part of the functionality of
      HttpMethodBase, contrary to the previous point, do they also
      belong on HttpMethod?  If not, why not?
    * Is the proxy server sending any indication that it is closing the
      connection?  If so, various parts of HttpMethodBase _should_
      already be detecting that the connection should be closed.  If
      HttpMethodBase is not detecting that the connection should be
      closed, why not?
    * Since the if condition you altered near the end of executeMethod()
      exists to consume the remainder of the response, or close the
      connection, based on the response stream, it would seem that your
      changes are needed when the responseStream is null (and perhaps
      shouldn't be?), or when the connection is being closed by the
      server without telling the client.  In the latter case, I would
      think it more appropriate to have a "recoverable" exception
      thrown, which exists for precisely those scenarios where the
      server "unexpectedly" closes the connection - which is to say that
      the connection dropped without any notification from the server
      that it was going to do so.

-Eric Johnson

Rob Di Marco wrote:

>We have recently experienced a problem when making an HTTP request through a 
>proxy server when the response was a 302 - Redirect.  The HttpMethodBase 
>object would attempt to use the same HttpConnection instance to make the 
>redirect request.  However, the proxy server was closing the connection 
>before the request could be made, so no data is read for the request, 
>resulting in an HttpRecoverableException being thrown.
>
>The solution to this problem was to add a field to the HttpMethodBase object,
>reinitializeConnectionOnRetry and by forcing the connection to close anytime a 
>retry is warranted.
>
>I have attached the patch to the message.
>
>Thanks.
>  
>
>------------------------------------------------------------------------
>
>Index: src/java/org/apache/commons/httpclient/HttpMethodBase.java
>===================================================================
>RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
>retrieving revision 1.96
>diff -u -r1.96 HttpMethodBase.java
>--- src/java/org/apache/commons/httpclient/HttpMethodBase.java	23 Jan 2003 22:47:47 -0000
1.96
>+++ src/java/org/apache/commons/httpclient/HttpMethodBase.java	24 Jan 2003 18:55:44 -0000
>@@ -252,6 +252,7 @@
> 
>     private boolean doneWithConnection = false;
> 
>+    private boolean reinitializeConnectionOnRetry = false;
>     //~ Constructors ···························································
> 
>     // ----------------------------------------------------------- Constructors
>@@ -366,6 +367,26 @@
>     }
> 
>     /**
>+     * Whether connections should be reinitialized across redirects.
>+     *
>+     * @param reinitializeConnectionOnRetry true to close and reopen a 
>+     * connection, false otherwise.
>+     */
>+    public void setReinitializeConnectionOnRetry(boolean reinitializeConnectionOnRetry)
{
>+        this.reinitializeConnectionOnRetry = reinitializeConnectionOnRetry;
>+    }
>+
>+    /**
>+     * Whether or not I should reconnect before following HTTP redirects
>+     * (status code 302, etc.)
>+     *
>+     * @return <tt>true</tt> if I will reconnect before follow HTTP redirects
>+     */
>+    public boolean getReinitializeConnectionOnRetry() {
>+        return this.reinitializeConnectionOnRetry;
>+    }
>+
>+    /**
>      * Set whether or not I should use the HTTP/1.1 protocol.
>      *
>      * @param http11 true to use HTTP/1.1, false to use 1.0
>@@ -929,7 +950,19 @@
>                 // retry - close previous stream.  Caution - this causes
>                 // responseBodyConsumed to be called, which may also close the
>                 // connection.
>-                if (responseStream != null) {
>+
>+                if (reinitializeConnectionOnRetry) {
>+
>+                    if (log.isDebugEnabled()) {
>+
>+                        log.debug("Closing the connection");
>+
>+                    }
>+
>+                    conn.close();
>+                }
>+
>+                if (conn.isOpen() && responseStream != null) {
>                     responseStream.close();
>                 }
> 
>
>  
>
>------------------------------------------------------------------------
>
>--
>To unsubscribe, e-mail:   <mailto:commons-httpclient-dev-unsubscribe@jakarta.apache.org>
>For additional commands, e-mail: <mailto:commons-httpclient-dev-help@jakarta.apache.org>
>


Mime
View raw message