commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d...@multitask.com.au
Subject Re: httpmethodbase refactoring
Date Sun, 04 Aug 2002 05:27:55 GMT
jsdever wrote on 08/04/2002 01:32:10 PM:

> 
> A little farther down I had added:
>         //exceeded max number of retries
>         throw new HttpException("Unable to process request: maximum 
number of
> retries (" +
>                 maxRetries + ") exceeded.");
> which executed if the loop ended (no return hit, no exception thrown in 
the
> loop).  But really that behaviour is not as obvious as it was before.
Yep, and the original exception that caused the last retry to fail is lost 
to the caller.

> IThere is somthing that still bothers me about that method.  Like if the
> write succeeds, but the read fails with a recoverable exception,  the
> connection is closed and opened again which is OK, but the write will 
proceed
> again.
> 
> I took another shot at it by incorporating your comments and spliting 
out the
> read and write operations into seperate retry loops.  Its a lot longer, 
but
> what do you think about this:
> 
>     /**
>      * Write a request and read the response.
>      *
>      * Both the read and the write will be retried {@link #maxRetries}
>      * times if an operation fails with a HttpRecoverableException.
>      * The write will only be attempted if the read has succeeded.
>      * <p>
>      * The <i>used</i> is set to true if at least the write succeeds.
>      *
>      * @param state the current state
>      * @param connection the connection for communication
>      * @throws HttpException when errors occur as part of the HTTP 
protocol
>      *      conversation
>      * @throws IOException when an I/O error occurs communicating with 
the
>      *      server
>      * @see writeRequest(HttpState,HttpConnection)
>      * @see readResponse(HttpState,HttpConnection)
>      */
>     private void processRequest(HttpState state, HttpConnection 
connection)
>     throws HttpException, IOException {
>         log.trace("enter HttpMethodBase.processRequest(HttpState,
> HttpConnection)");
> 
>         //try to do the write
>         for (int retryCount = 1; retryCount <= maxRetries; retryCount++) 
{
>             if (log.isTraceEnabled()){
>                 log.trace("Attempt number " + retryCount + " to write
> request");
>             }
>             try {
>                 if (!connection.isOpen()) {
>                     log.debug("Opening the connection.");
>                     connection.open();
>                 }
>                 writeRequest(state, connection);
>             } catch (HttpRecoverableException httpre) {
>                 log.debug("Closing the connection.");
>                 connection.close();
>                 log.warn("Recoverable exception caught when writing
> request");
>                 if (retryCount == maxRetries) {
>                     log.warn("Attempt to write request has reached max
> retries: " +
>                             maxRetries);
>                     throw httpre;

We could just store the exception away for later use rather than throwing 
it here.

>                 }
>             }
>         }

We could re throw it here for example...?

>         //at least the write has succeeded
>         used = true;
> 
>         //try to do the read
>         for (int retryCount = 1; retryCount <= maxRetries; retryCount++) 
{
>             if (log.isTraceEnabled()){
>                 log.trace("Attempt number " + retryCount + " to read
> response");
>             }
>             try {
>                 if (!connection.isOpen()) {

If the connection isn't open after the write, we aren't going to get a 
sane response to read......retrying just the read doesn't seem to add 
anything extra here....

[snip]
--
dIon Gillard, Multitask Consulting
Work:      http://www.multitask.com.au
Developers: http://adslgateway.multitask.com.au/developers



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message