Mailing-List: contact dev-help@hc.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "HttpComponents Project" <dev@hc.apache.org>
Message-ID: <26667685.73271295490043709.JavaMail.jira@thor>
Date: Wed, 19 Jan 2011 21:20:43 -0500 (EST)
From: "Steffen Yount (JIRA)" <jira@apache.org>
To: dev@hc.apache.org
Subject: [jira] Reopened: (HTTPCLIENT-1044) DefaultHttpRequestRetryHandler
 is not handling PUT as an idempotent method for retries, though RFC2616
 section 9.1.2 clearly defines it to be one.
In-Reply-To: <9264311.381671295057866256.JavaMail.jira@thor>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


     [ https://issues.apache.org/jira/browse/HTTPCLIENT-1044?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Steffen Yount reopened HTTPCLIENT-1044:
---------------------------------------


Hi Oleg,

Thanks for considering my patch, and sorry for the long turn around in getting back to you.

h4. I think there is value in uniform application/non-application of the RFC's rules here:

To demonstrate what I mean, I believe I can make a statement equally valid to your first comment with respect to the GET method.

"While GET methods should be limited to actions of retrieval and be idempotent, in real life often they are not. The current implementation may already be breaking existing applications in very nasty ways."

Some common examples of GET method usage diverging from spec in this way: html form submissions, RPC based APIs over HTTP, tracking pixel requests, JSONP APIs (hack for cross domain scripting limitations).

I don't think the RFC's idempotence rules should be selectively respected on some HTTP methods and not on others (without really good reasons). "Do or do not" as Yoda would say. :-)

The current implementation retries GET, HEAD, DELETE, OPTIONS, and TRACE assuming idempotence.


h4. It is better to keep the default implementation more idiot-proof:

I agree with your assessment that assuming idempotence and unintentionally retrying non-idempotent requests can break applications in "very nasty ways". 

The nastiest of these ways are in debugging what went wrong, since a) the failure modes that lead to retries are rarely replicated or tested by app developers and b) many novice developers lack the insight to understand the value of idempotent implementations and to identify idempotence/non-idempotence related bugs.

Considering that automatic retries are optional as far as the RFC is concerned and considering the two points above, I'm on board with your assessment that the default behavior of the DefaultHttpRequestRetryHandler shouldn't be to retry PUT methods automatically.

But, I also think this reasoning should be taken one step further in the interest of uniformity and idiot-proofing: the default behavior shouldn't be using RFC defined method idempotence to enable/disable automatic retries on any of the HTTP methods.


h4. Still, I think it would be great for the HttpClient libraries to provide an out of the box HttpRequestRetryHandler that uses the RFC2616's rules for 

HTTP method idempotence to enable/disable automatic retries.

Sure, it is true that I can use a custom implementation of HttpRequestRetryHandler. But the supported behavior I'm asking for here isn't something crazy. It's something that should work in apps that respect the RFC2616's rules for HTTP method idempotence, and it is something that could be provided out of the box. 


h4. I've provided 2 new alternative patches to make changes in this direction, please let me know if either one is acceptable. 

The changes in "HTTPCLIENT-1044_planB.patch" are:
# Defaulting to not retry GET, HEAD, DELETE, OPTIONS, and TRACE based on assumed idempotence.
# Adding a check to prevent resending unrepeatable entities that have already been sent once.
# Removing the "requestSentRetryEnabled" flag which seemed to be included for specifying entity repeatability and allowed both PUT and POST methods to be resent.
# Adding a "retryIdempotentMethods" flag in place of the "requestSentRetryEnabled" flag. This flag allows GET, HEAD, PUT, DELETE, OPTIONS, and TRACE methods to be resent. (default value: false) 

The changes in "HTTPCLIENT-1044_planC.patch" are:
# Defaulting to not retry GET, HEAD, DELETE, OPTIONS, and TRACE based on assumed idempotence.
# Adding a check to prevent resending unrepeatable entities that have already been sent once.
# Adding a new DefaultIdempotentHttpRequestRetryHandler class which does allow GET, HEAD, PUT, DELETE, OPTIONS, and TRACE methods to be resent.

Cheers,
-Steffen

> DefaultHttpRequestRetryHandler is not handling PUT as an idempotent method for retries, though RFC2616 section 9.1.2 clearly defines it to be one.
> --------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HTTPCLIENT-1044
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1044
>             Project: HttpComponents HttpClient
>          Issue Type: Bug
>    Affects Versions: 4.0.3, 4.1.0
>            Reporter: Steffen Yount
>         Attachments: HTTPCLIENT-1044.patch
>
>
> See attached patch file for a fix:
> Fix treats PUT requests as idempotent, marking them to be retried when their enclosed HttpEntity is either null or repeatable.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org