Hi Mohammad,
Once again, thanks a bunch for taking your time to review HttpClient
3.0a1. Until now you are pretty much the only one who gave us some
feedback on 3.0 API, which makes your contribution really appreciated
For specific points, see my comments in-line
> > 2) The deprecated set parameter methods in HttpClient seem to set the
> > parameters on the connection manager. Why? Shouldn't those be set on the
> > HttpClientParams (specifically, see setSoTimeout()).
>
> I might be just an oversight on my part. I'll look into this
Ok. I believe I recall the rationale for that. At the time of writing of
the new preference architecture, socket timeout & connect timeout were
believed to represent connection / connection manager properties. Later
on we figured that socket timeout setting would also make sense as a
HTTP method property, because HTTP connections were not directly exposed
to the user. HttpClientParams#setSoTimeout method was introduced to work
the problem around.
So, currently
* The connection level socket timeout represents the default value
settable upon connection initialization.
* The optional method level socket timeout represents the value settable
upon method execution which overrides the default one.
The old 2.0-style socket and connect timeout settings correspond to
those of the connection manager (at least IMO).
Let me now if you think that all this stuff does (or does not) make any
sense
>
> > 3) Minor point: We got an IO timeout exception and it didn't have the
> > underlying cause. I tracked it down to
> > HttpConnection.WrappedInputStream.handleException: the exception constructor
> > is not passing in the cause. Trivial to fix, but maybe a short review of the
> > code is in order to find such instances.
>
> Good catch. I'll review the exception handling code in the HttpConnection class and
> fix what's broken.
>
I reviewed the code found only two instances where the root exception
was not propagated to the caller
HttpConnection.WrappedInputStream.handleException
HttpConnection.WrappedOutputStream.handleException
I'll submit the following corrective patch shortly
Index: HttpConnection.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v
retrieving revision 1.92
diff -u -r1.92 HttpConnection.java
--- HttpConnection.java 1 Jun 2004 00:55:10 -0000 1.92
+++ HttpConnection.java 14 Jun 2004 19:29:32 -0000
@@ -1249,12 +1249,12 @@
* @param ioe the exception that occurred
* @return the exception to be thrown
*/
- private IOException handleException(IOException ioe) {
+ private IOException handleException(final IOException ioe) {
// keep the original value of used, as it will be set to
false by close().
boolean isRecoverable = HttpConnection.this.used;
HttpConnection.this.close();
if (ioe instanceof InterruptedIOException) {
- return new IOTimeoutException(ioe.getMessage());
+ return new IOTimeoutException(ioe.getMessage(), ioe);
} else if (isRecoverable) {
LOG.debug(
"Output exception occurred on a used connection.
Will treat as recoverable.",
@@ -1333,9 +1333,9 @@
* @param ioe the exception that occurred
* @return the exception to be thrown
*/
- private IOException handleException(IOException ioe) {
+ private IOException handleException(final IOException ioe) {
if (ioe instanceof InterruptedIOException) {
- return new IOTimeoutException(ioe.getMessage());
+ return new IOTimeoutException(ioe.getMessage(), ioe);
} else {
return ioe;
}
Cheers
Oleg
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
