On 8/29/23 3:48 AM, Oleg Kalnichevski wrote:
This code is not about connection clean-up, this code is about
connection re-use. The resource clean-up is ensured by closing out the
response.
Ok, but then why is EntityUtils.consume(entity) being called at all?
Seems like it's either necessary to explicitly close the HttpEntity
content stream at the end of request processing, or it's not
necessary. If it is necessary, we're pointing out that it's not always
being done. If it's not necessary, then what is the point of calling
EntityUtils.consume(entity), most especially in the catch block for the
checked exception?
It's the inconsistency that we're asking about.
Or I can also just turn the question around and ask: When is it
necessary to ensure the request's HttpEntity is closed, such as by
calling EntityUtils.consume(entity)?
One should almost never, never, ever catch Throwable.
I would in general agree. But I think an exception (no pun intended)
would be for example when you need to intercept the failure and perform
some necessary cleanup before propagating the error back out.
To be clear, I'm not suggesting that a Throwable or other unchecked
type would be either swallowed or turned into a checked exception. I
would certainly re-throw those types to preserve fidelity with how the
response handler originally failed.
Again, this is about trying to salvage and re-use the connection, not
about resource clean-up. Besides, what usually all entities do in their
#close method is closing their content stream.
(Aside from my question above about exactly when HttpEntity and/or its
stream must be closed) The thing we're focused on is the "usually' in
your sentence. It's not a given that all future HttpEntity
implementations - or custom types that a user might create - will
*only* need to close their content stream. Since HttpEntity is
Closeable, it knows how to do the right thing to close itself, so it
just seems odd that that is not being used here. Introspecting by
checking for streamable and then closing (just) the entity content
stream risks something being missed that the HttpEntity would
(presumably) itself know how to do properly in its own #close().
Thanks,
Brent
