hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steffen Yount (JIRA)" <j...@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.
Date Thu, 20 Jan 2011 02:20:43 GMT

     [ 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


Mime
View raw message