http-useragent
http-useragent copied to clipboard
Move responsibility for throwing exceptions to HTTP::Response
In #95 @moritz suggested
Another idea would be to throw an exception when accessing the contents of a request with 4* or 5 status without checking the status code first.
Which seems like a very good idea, it moves the responsibility for knowing what is a good response into a better place and should make the code clearer.
I would however like to wait until after #97 as that will almost certainly involve some more immediately obvious movements between HTTP::UserAgent and HTTP::Request and HTTP::Response.
HI @jonathanstowe & @moritz :)
Now that #97 is closed, what do you think of this issue?
As an HTTP::UserAgent user, I think consistency is what's most important. I have my concerns regarding methods that behave differently depending on potentially distant actions such as checking the status code. While I do understand how most users might simply try and handle the contents without paying a second thought to the status code, and how raising an exception feels like the right thing to do in such cases, what I - as a user - think will happen is people will just put status verification in all their code because "I don't know, sometimes content() just blows up on my face".
To me, an exception when using HTTP::UserAgent is a connection error outside the application layer, like a network or protocol issue. A 4** status is a valid response and could bring valid content. If HTTP::UserAgent is biting the exception bullet (and, by extension, so are HTTP::{Request|Response}), I think it might be better to enforce checking the status code - or is-success()
- before accessing the contents of a response regardless of its status, instead of only 4** or 5** responses. Or skip raising the exception altogether unless something like :throw-exceptions
is explicitly used, as suggested by @tokuhirom, @azawawi and @sergot in #95.
Anyway, just some thoughts thrown in. Maybe I missed something :)
Cheers!
Hi,
As it stands now unless :throw-exceptions
is provided to the constructor an exception won't be thrown if enough of the response is returned to determine what the intent of the server was, if there is no response whatsoever or it returns garbage then an exception will be thrown as we don't know what the intent of the server was. That is to say it won't throw an exception based on the response code.
I'm quite relaxed about it personally, this ticket is mostly to encourage discussion :)