Sam Maloney a écrit :
>
> Makes sense to me,
> I would definatly agree on your point that 'Client' logic should be in
> HttpClient and not in HttpMethodBase. (I would say redirect, auth and
> even auto-retry would count as 'Client' logic).
>
> Sam

As for the auto-retry stuff, I agree it makes the HttpMethodBase really ugly
to read, I got lost several times trying to understand where my exception
camed from.

Wouldn't it be possible to clean the execute method (to remove extra stuff,
and keep it clear to read) and then add another method (in fact the method
is what Sam suggests to do when making requests in a app...) :

public int execute(HttpState state, HttpConnection conn, int maxRetries)
throws HttpException, IOException, NullPointerException
{
  for(int i=0, i<maxRetries, i++)
  {
        try
        {
          execute(state, conn);
          break; // No exception
        }
        catch (HttpRecoverableException re)
        {
          if (i == maxRetries)
          {
                throw new HttpException("Attempt to execute request has reached max
retries: "
                        + maxRetries)
                // HttpException ? cause it really failed :)
          }
        }
  }
}

Cause with current CVS if an exception occurs reading response, the
exception is going back to main thread, and even worse the connection isn't
released (cause responseStream is null, so releaseConnection() does
nothing), leaving connectionManager in a wrong state :(

As I don't
As for the auto-retry stuff, I agree it makes the HttpMethodBase really ugly
to read, I got lost several times trying to understand where my exception
camed from.

Wouldn't it be possible to clean the execute method (to remove extra stuff,
and keep it clear to read) and then add another method (in fact the method
is what Sam suggests to do when making requests in a app...) :

public int execute(HttpState state, HttpConnection conn, int maxRetries)
throws HttpException, IOException, NullPointerException
{
  for(int i=0, i<maxRetries, i++)
  {
        try
        {
          execute(state, conn);
          break; // No exception
        }
        catch (HttpRecoverableException re)
        {
          if (i == maxRetries)
          {
                throw new HttpException("Attempt to execute request has reached max
retries: "
                        + maxRetries)
                // HttpException ? cause it really failed :)
          }
        }
  }
}

Cause with current CVS if an exception occurs when reading response, the
exception is going back to main thread, and even worse the connection isn't
released (cause responseStream is null, so releaseConnection() does
nothing), leaving connectionManager in a wrong state :(


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

Reply via email to