hc-httpclient-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Hosier <hosier.da...@me.com>
Subject Re: Broken BasicResponseHandler
Date Thu, 06 Oct 2011 22:13:43 GMT
Ok, thanks. I'm taking a stab at a patch right now.

-- David Hosier

On Thursday, October 6, 2011 at 2:52 PM, Oleg Kalnichevski wrote:

> On Thu, 2011-10-06 at 14:23 -0700, David Hosier wrote:
> > On Thursday, October 6, 2011 at 1:40 PM, Oleg Kalnichevski wrote:
> > > On Thu, 2011-10-06 at 13:21 -0700, David Hosier wrote:
> > > > Understood. The library does not support the use case of obtaining the
entity of a response via the recommended usage for any response other than a 2xx.
> > > 
> > > BasicResponseHandler does not, hence, unsurprisingly, the name Basic.
> > > However, there is _nothing_ that prevents you from writing a custom,
> > > better ResponseHandler which handles response entities differently.
> > > While the ResponseHandler interface is indeed the recommended way of
> > > handling responses, the BasicResponseHandler is just its very basic
> > > implementation which was never meant to be used for anything else but
> > > the most simplistic use cases. No one in their sane mind should _ever_
> > > convert an HTTP response to a string in a productive application. 
> > > 
> > > >  Additionally, the javadoc for BasicResponseHandler is incorrect. 
> > > 
> > > What exactly is incorrect? If you think javadocs are not clear enough or
> > > specific enough I'll happily apply a patch if you submit one.
> > I went and looked more closely, and the issue is that I was looking at the class-level
javadoc for BasicResponseHandler. The class-level javadoc indicates that the response body
is read before throwing the exception on status codes >=300. However, the javadoc for the
handleResponse() method does not indicate that the response body is read. The statement about
reading the response body on >=300 really only occurs when used with HttpClient, and it's
that class that actually does the reading. That's how I read the code at least.
> 
> Fair enough. This may sound misleading if taken out of the usual context
> of always using ResponseHandler together with HttpClient. I'll tweak the
> javadocs tomorrow (unless you would like to submit a patch yourself)
> 
> Oleg
> 
> > > Oleg
> > > 
> > > > So now that I understand better how things work, I can take action accordingly.
Thanks for the responses.
> > > > 
> > > > -- David Hosier
> > > > 
> > > > On Thursday, October 6, 2011 at 11:40 AM, Oleg Kalnichevski wrote:
> > > > 
> > > > > On Thu, 2011-10-06 at 11:31 -0700, David Hosier wrote:
> > > > > > I'm using the DefaultHttpClient to make the call, yes. I want
to use DefaultHttpClient with the ResponseHandler the way I am supposed to. However, the API
does not give me the ability to get a hold of the Entity if the status code is 404, because
it throws an Exception which does not contain the entity value. I need the Entity value, even
if the call returns 404. As far as I can tell, I cannot get the information I need from the
API the way it is designed to be used. Is that clearer? Is my assessment correct?
> > > > > 
> > > > > Yes, it is intentional that the exception thrown does not contain
a
> > > > > response body, because it would involve reading the entire body content
> > > > > into a memory buffer.
> > > > > 
> > > > > Oleg
> > > > > 
> > > > > 
> > > > > > -- David Hosier
> > > > > > 
> > > > > > On Thursday, October 6, 2011 at 11:29 AM, Oleg Kalnichevski
wrote:
> > > > > > 
> > > > > > > On Thu, 2011-10-06 at 10:47 -0700, David Hosier wrote:
> > > > > > > > I am using this to interface with some REST services.
One key to a good REST service is to never let something like a 404 spit out the server's
generic 404 HTML page in response to a REST request. So my service instead returns an entity
with the 404 that says something like "Could not find alert 12334". I should be able to show
this response entity. However, given the way the ResponseHandler works with HttpClient, this
is not possible, because the entity is not part of the Exception that is thrown when the ResponseHandler
encounters a 404. Without manually reading the entity after ResponseHandler throws an Exception,
I would only be able to show the fields that are contained in the Exception. That means I
could only show the text 'Not Found', which is hardly meaningful since the status code of
404 already tells me that.
> > > > > > > 
> > > > > > > You are using ResponseHandler to interface with some REST
services
> > > > > > > without using DefaultHttpClient? I am sorry but it still
makes no sense
> > > > > > > to me. You might as well handle responses from that service
_any_ way
> > > > > > > you like without using a ResponseHandler.
> > > > > > > 
> > > > > > > Oleg
> > > > > > > 
> > > > > > > > -- David Hosier
> > > > > > > > 
> > > > > > > > On Thursday, October 6, 2011 at 5:39 AM, Oleg Kalnichevski
wrote:
> > > > > > > > 
> > > > > > > > > On Thu, 2011-10-06 at 02:59 -0700, David Hosier
wrote:
> > > > > > > > > > Ok, I see what the difference is in this
situation. I am not passing the ResponseHandler to the execute() method. I am actually calling
handleResponse() on the ResponseHandler manually.
> > > > > > > > > 
> > > > > > > > > I honestly see no sense in doing so. ResponseHandler
is pretty much
> > > > > > > > > useless without the resource management code
in AbstractHttpClient.
> > > > > > > > > 
> > > > > > > > > What is the reason you want to invoke #handleResponse
manually?
> > > > > > > > > 
> > > > > > > > > Oleg
> > > > > > > > > 
> > > > > > > > > >  The problem I have with the implementation
is that I return error messages on error conditions. With the way this works, you can only
get very basic information from the HttpResponseException. For example, on a 404, it looks
like the Exception only contains 404 and 'Not Found'. I am able to pluck out the entity when
invoking handleResponse() manually by simply consuming the entity myself, but it's not possible
to get the entity if the ResponseHandler is passed to execute() and the status is not 2xx.
Am I off base here or is my analysis correct? Would you recommend that if I really need the
entity on a non-2xx response that I just keep manually consuming the entity? I'm not sure
it would make sense for your library to attempt to consume the entity in BasicResponseHandler
and try to add it as an
> > > > > > > > > >  other fi
> > > > > > > > > > eld to the HttpResponseException. The AbstractHttpClient
code you linked me to would have to change if you did that.
> > > > > > > > > > 
> > > > > > > > > > -- David Hosier
> > > > > > > > > > 
> > > > > > > > > > On Thursday, October 6, 2011 at 2:30 AM,
David Hosier wrote:
> > > > > > > > > > 
> > > > > > > > > > > On Thursday, October 6, 2011 at 2:22
AM, Oleg Kalnichevski wrote:
> > > > > > > > > > > > On Wed, 2011-10-05 at 13:44 -0700,
David Hosier wrote:
> > > > > > > > > > > > > Perhaps I'm wrong, but the
code for BasicResponseHandler in httpclient 4.1.2 does not satisfy the javadocs as written.
The javadoc states the following:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > "If the response code was
>= 300, the response body is consumed and an HttpResponseException (http://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/client/HttpResponseException.html)
is thrown."
> > > > > > > > > > > > > 
> > > > > > > > > > > > > However, the code does not
do that:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > StatusLine statusLine = response.getStatusLine();
> > > > > > > > > > > > > if (statusLine.getStatusCode()
>= 300) {
> > > > > > > > > > > > >  throw new HttpResponseException(statusLine.getStatusCode(),
> > > > > > > > > > > > >  statusLine.getReasonPhrase());
> > > > > > > > > > > > > }
> > > > > > > > > > > > > 
> > > > > > > > > > > > > HttpEntity entity = response.getEntity();
> > > > > > > > > > > > > return entity == null ? null
: EntityUtils.toString(entity);
> > > > > > > > > > > > > 
> > > > > > > > > > > > > The code clearly throws the
Exception without reading the entity. So what happens is that if you get a non-2xx response,
connections are never released as can be seen by enabling DEBUG logging for the library. Am
I misreading the code or javadocs, or is this really broken? If I catch the Exception and
then read the entity manually like shown above, I can see the connections being closed.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > -David
> > > > > > > > > > > > 
> > > > > > > > > > > > Hi David
> > > > > > > > > > > > The resource management is taken
care of by HttpClient [1]. I do not
> > > > > > > > > > > > think BasicResponseHandler is
broken. The whole point of ResponseHandler
> > > > > > > > > > > > is to free the user from having
to worry about resource management and
> > > > > > > > > > > > response entities.
> > > > > > > > > > > Interesting. Thanks for the link to
the code. I can assure you that in my situation however, that the connections are not getting
closed. I'll take a closer look at the code and compare it to this linked code to see if I'm
using the right stuff. My assumption at this point then is that I'm just doing something wrong.
Thanks. 
> > > > > > > > > > > > 
> > > > > > > > > > > > Oleg
> > > > > > > > > > > > 
> > > > > > > > > > > > [1]
> > > > > > > > > > > > http://hc.apache.org/httpcomponents-client-ga/httpclient/xref/org/apache/http/impl/client/AbstractHttpClient.html#930
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > > ---------------------------------------------------------------------
> > > > > > > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > > > > > > For additional commands,
e-mail: httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > ---------------------------------------------------------------------
> > > > > > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > > > > > For additional commands, e-mail:
httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > ---------------------------------------------------------------------
> > > > > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > ---------------------------------------------------------------------
> > > > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > ---------------------------------------------------------------------
> > > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ---------------------------------------------------------------------
> > > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ---------------------------------------------------------------------
> > > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > > 
> > > > > 
> > > > > 
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org
(mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org
(mailto:httpclient-users-help@hc.apache.org)
> > > > 
> > > > 
> > > > 
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org (mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > > For additional commands, e-mail: httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)
> > > 
> > > 
> > > 
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org (mailto:httpclient-users-unsubscribe@hc.apache.org)
> > > For additional commands, e-mail: httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)
> > 
> > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org (mailto:httpclient-users-unsubscribe@hc.apache.org)
> > For additional commands, e-mail: httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: httpclient-users-unsubscribe@hc.apache.org (mailto:httpclient-users-unsubscribe@hc.apache.org)
> For additional commands, e-mail: httpclient-users-help@hc.apache.org (mailto:httpclient-users-help@hc.apache.org)



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


Mime
View raw message