hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 19868] - Exception handling in HttpClient requires redesign
Date Tue, 15 Jul 2003 20:06:11 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG 
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://nagoya.apache.org/bugzilla/show_bug.cgi?id=19868>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND 
INSERTED IN THE BUG DATABASE.

http://nagoya.apache.org/bugzilla/show_bug.cgi?id=19868

Exception handling in HttpClient requires redesign





------- Additional Comments From becke@u.washington.edu  2003-07-15 20:06 -------
> I played around with several approaches and realized that wrapping all
> IOExceptions into HttpTransportExceptions was not simply feasible, primarily
> because InputStream / OutputStream derived classes cannot throw
> non-IOExceptions. I found no way to work that problem around in an acceptable
manner

Makes sense.
 
> The patch basically implements the first scenario of my original proposal and
> goes along the line suggested by Adrian: 
> * IOException represents transport exceptions
> * HttpException represents protocol exceptions
> 
> Here's the list of important points
> * TransportException no more

Adrian wins:)

> * HttpException is no longer derived from IOException (which is good)

I'm not so sure this is a good thing. This adds up to a very large API change
for many users.  People who are wrapping HttpClient.executeMethod() in a try{
}catch(IOException) are going to be in for some trouble.  I think we should
leave the IOException inheritence of now (even though it is ugly).

> * URIException is no longer derived from IOException (which is good)

This is only okay I think if we keep HttpException as an IOException for similar
reasons as above.

> * I had to deprecate HttpConnectionManager#getConnection(HostConfiguration,
> long) because of wrong exception type it throws

Looks good to me.  Adding the getConnectionWithTimeout() method breaks the API
but I do not think may people are creating their own connection mangers.

> * In ChunkedInputStream I had to implement a pretty ugly hack to work around
> exception type mismatc

That's not so bad.  I think we should add an error log since the original stack
track will be lost.

> * HttpConnection can now throw IOTimeoutException

Jolly good.


Overall this patch is a good incremental change.  The notable exception being
the change to HttpException.  This is a pretty big change and I do not think it
is appropriate for 2.1.  I suggest we keep HttpException as an IOException and
make it very clear (in JavaDocs, release notes, etc.) that it will not be that
way for 3.0.

Mike

Mime
View raw message