Re: [PATCH] Closing HttpConnection before retrying

[Eric Johnson]

Rob,

OK, your description helps. I'm trying to step through the code in my head, though, trying to figure out where things are going wrong, as you didn't describe the HttpClient routines that fail. I augmented your steps through the failure case to see if I can understand why it doesn't work.

Rob Di Marco wrote:

The situation causing the problem is the following:

1) The HttpClient makes a connection to the proxy server for a site.

2) The proxy server connects to the remote web server and gets a 302
redirect response

- aka. HttpMethodBase.execute() calls writeRequest(), followed by readResponse(). Of course, you didn't mention whether this was a "GET", "HEAD", "POST", or some other request, which could matter.

3)  The proxy server returns the 302 response back to the HttpClient
4)  The proxy server closes the socket between it and the HttpClient.

- at this point HttpClient is oblivious, the execute() method continues, invoking isRetryNeeded() - which returns "true".

5)  The HttpClient assumes that the connection is still open and tries to
make the redirect down the same connection

execute() proceeds apace, and invokes responseStream.close() - still no exceptions thrown in any of this behavior so far. (Or is this throwing an exception in your scenario?!)

6)  Once the connection to the proxy is closed no data can be written to the
proxy or read from the next request.  This results in the exception being
thrown.

Since execute() still has a "forwardCount" less than "maxForwards", it loops around, invoking processRequest again. As you note, processRequest() calls writeRequest(), which (correctly!) throws an HttpRecoverableException (It is unclear to me whether you are complaining that an exception is thrown, which I think is correct behavior in your described scenario, or rather that an exception is thrown, but not caught?).

At this point, I become confused. I expect, at this point, that processRequest() would catch the exception, close the connection, and loop again, reopening the connection and retrying the request, and this time it would succeed. Based on your writeups, however, I'm wondering whether things are out of whack at this point, and the sequence of events is different than what I expect would occur.

I would not think that the client should assume that the connection
automatically closes as this is not the case for the vast majority of
servers that it would connect to. One of the problems with the recoverable
exception is that is telling me that I am failing, but I am failing on the
redirect rather than on the initial request. So if I try the exact same
request again, the exception will still occur.

Understood. I agree there is a bug here based on what you've described, I'm just concerned that the failure is more subtle than you've uncovered so far.

So for a particular request, I do not explicity know that I will need a
redirect until I get the initial response from the external server. But I
do know that if I do get such a response, I want to make a new connection to
the server. You are correct in saying the reason that I know this is
because I am setting up the connection to the proxy server, I see your point
there. But it seems odd to me to have that attribute in the connection. I
guess I look at the attributes of HttpConnection as those pieces of data
involved in making the connection rather than being involved with decisions
about whether the connection should be open or closed. That is why I still
think that the value belongs in the HttpMethodBase. If the methods are
defined in this class, I think HttpMethod should be modified as well for the
sake of consistency as the other attributes of the class are defined in the
interface.

That HttpMethod is already too large an interface cuts strongly against your proposed solution, for the very reason that the HttpMethodBase class and HttpMethod interface should be in sync.

As of yet, I haven't written any test cases for this problem.

Perhaps you can provide the stack trace of the exception failure, or a more specific "trace" of the scenario, mentioning routine names (and line numbers?) as I have attempted to do based on your description? You might also consider including a "wire" level debug trace, and/or snooping on the connection (see the mailing list archive for tips on how you might do this). Maybe, you could augmenting SimpleHttpConnection so that it can mimic the unexpected close for you, and make your case 100% reproducible for others.

-Eric.


--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>