Very nice work Laura. Thank you for taking the initiative on this one. Please see the in-lined comments below.

I'm attaching a preliminary patch for this, following Oleg's proposal on the
mailing list. Here are some areas where I'd like feedback:


- Is AuthenticationException too broad? Should there be different exceptions for:
- Incorrect credentials / password / login / whatever, vs.
- Other problem: no such provider, unknown auth type, ..

Possibly. I could see this extra information as quite useful. My only worry would be that we have too many exception types.


- I added an HttpInterruptedException for cases where client code tells us to
abort a transaction.
- Should I leave this out until we actually add the abort code?
- Should it be called HttpAbortedException instead? Or something else?

I think we will probably need it but I would say to leave it out for now. We can always add it when the time comes.


I think HttpAbortedException is better.

- I'm starting to wonder if all these classes really need "Http" at the
beginning of their names. HttpException definitely does, for compatibility
reasons if nothing else. But could the others just be ProtocolException,
TransportException, etc? There's a chance for a collision if a Java file uses
two different network libraries that both have a ProtocolException, but it
doesn't seem to likely.

I say lose the Http. It seems a little redundant. No worries about name collision. That's the beauty of packages.


- I added a primitive exception chaining mechanism to HttpTransportException.
Right now it wraps any Throwable. Oleg had suggested wrapping just IOException.
Any preferences? Also, how fancy does the chaining mechanism need to be? I'm
happy with just "getCause()", but if people want me to I could add some of the
other exception chain accessors that commons-lang uses in its classes.

I think we should push the cause all the way up to the HttpException level. Nestable exceptions occur in other places than just IO. Your TODO example of NTLM IllegalBlockSize is a good one. I imagine there will be a few involving URIExceptions as well.


I think leaving just getCause() for now is fine. The most important part is getting the hierarchy right. We can add bells and whistles at a later time.

Now that we have this organized a little better (thank you Laura), how do we imagine that a user of HttpClient will take advantage of these exceptions? If we only throw an HttpException at the HttpClient level, how do we plan to expose the other exception types? Will we have examples that show all of the possible exceptions? Will there be a need for utility methods for parsing/handling the various errors? Just a few random questions. I haven't really thought through it yet.

Good night.

Mike


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to