Add error_for_status. #98
Conversation
| Err(::error::client_error(self.url().clone(), *self.status())) | ||
| } | ||
| StatusClass::ServerError => { | ||
| Err(::error::server_error(self.url().clone(), *self.status())) |
There was a problem hiding this comment.
Since this is consuming the Response, is there a way we can just take the Url, instead of cloning it?
There was a problem hiding this comment.
I don't think so. At the very least, we'd need to duplicate the body of Response::urlto get an owned version. But when I try that, I can't extract the url from the hyper::client::response::Response since that implements Drop.
src/error.rs
Outdated
|
|
||
| /// Returns the status code, if the error was generated from a response. | ||
| #[inline] | ||
| pub fn status(&self) -> Option<&::hyper::status::StatusCode> { |
There was a problem hiding this comment.
Since the StatusCode type is basically just a u32 (1 byte for tag, 2 for Unregistered(u16), 1 for padding), and it's Copy, may as well return it by value instead of by ref.
This is something that will be changed in Response::status() -> StatusCode as well, for the next breaking change round.
src/error.rs
Outdated
| @@ -58,11 +60,38 @@ impl Error { | |||
| #[inline] | |||
| pub fn is_redirect(&self) -> bool { | |||
| match self.kind { | |||
| Kind::TooManyRedirects | | |||
| Kind::RedirectLoop => true, | |||
| Kind::TooManyRedirects | Kind::RedirectLoop => true, | |||
| fmt::Display::fmt(code, f) | ||
| } | ||
| Kind::ServerError(ref code) => { | ||
| f.write_str("Server Error: ")?; |
There was a problem hiding this comment.
I'm imagining what the message would look like:
https://hyper.rs/not-here: Client Error: 404 Not Found
Is there value in the middle part, Client Error: or Server Error: , I wonder?
There was a problem hiding this comment.
I think so. I suspect that this will be shown to end-users who might not be familiar with the different HTTP status codes. (Note that I was just copying the python version here
src/error.rs
Outdated
|
|
||
| /// Returns true if the error is from a request returning a 4xx error. | ||
| #[inline] | ||
| pub fn is_client_error(&self) -> bool { |
There was a problem hiding this comment.
I wonder if there'd be confusing that this means a status.is_client_error(), and not an error from reqwest::Client.
There was a problem hiding this comment.
I haven't used reqwest enough to answer this.
tomprince
force-pushed
the
error-for-status
branch
3 times, most recently
from
77aebd6
to
bdfdaa4
Compare
This makes it it easy to turn error responses into error results.
tomprince
force-pushed
the
error-for-status
branch
from
bdfdaa4
to
184044a
Compare
|
I rebased this into master here 855e661 Thanks! |
This makes it it easy to turn error responses into error results.
This is inspired by python requests'
raise_for_status.