Add Error::{kind, status, into_response}.

Also, remove Response::{ok, error, client_error, server_error,
redirect}. The idea is that you would access these through the
Error object instead.

I fetched all the reverse dependencies of ureq on crates.io and looked
for uses of the methods being removed. I found none.

I'm also considering removing the error_on_non_2xx method entirely. If
it's easy to get the underlying response for errors, it would be nice to
make that the single way to do things rather than support two separate
ways of handling HTTP errors.
This commit is contained in:
Jacob Hoffman-Andrews
2020-11-25 20:15:46 -08:00
committed by Martin Algesten
parent 35c03521b9
commit 4c3b93d86d
5 changed files with 45 additions and 30 deletions

View File

@@ -13,7 +13,7 @@ pub struct Error {
message: Option<String>, message: Option<String>,
url: Option<Url>, url: Option<Url>,
source: Option<Box<dyn error::Error + Send + Sync + 'static>>, source: Option<Box<dyn error::Error + Send + Sync + 'static>>,
response: Option<Box<Response>>, response: Option<Response>,
} }
impl Display for Error { impl Display for Error {
@@ -67,13 +67,53 @@ impl Error {
} }
pub(crate) fn response(mut self, response: Response) -> Self { pub(crate) fn response(mut self, response: Response) -> Self {
self.response = Some(Box::new(response)); self.response = Some(response);
self self
} }
pub(crate) fn kind(&self) -> ErrorKind {
/// The type of this error.
///
/// ```
/// # ureq::is_test(true);
/// let err = ureq::get("http://httpbin.org/status/500")
/// .call().unwrap_err();
/// assert_eq!(err.kind(), ureq::ErrorKind::HTTP);
/// ```
pub fn kind(&self) -> ErrorKind {
self.kind self.kind
} }
/// For Errors of type HTTP (i.e. those that result from an HTTP status code),
/// return the status code of the response. For all other Errors, return 0.
///
/// ```
/// # ureq::is_test(true);
/// let err = ureq::get("http://httpbin.org/status/500")
/// .call().unwrap_err();
/// assert_eq!(err.kind(), ureq::ErrorKind::HTTP);
/// assert_eq!(err.status(), 500);
/// ```
pub fn status(&self) -> u16 {
match &self.response {
Some(response) => response.status(),
None => 0,
}
}
/// For an Error of type HTTP (i.e. those that result from an HTTP status code),
/// turn the error into the underlying Response. For other errors, return None.
///
/// ```
/// # ureq::is_test(true);
/// let err = ureq::get("http://httpbin.org/status/500")
/// .call().unwrap_err();
/// assert_eq!(err.kind(), ureq::ErrorKind::HTTP);
/// assert_eq!(err.into_response().unwrap().status(), 500);
/// ```
pub fn into_response(self) -> Option<Response> {
self.response
}
/// Return true iff the error was due to a connection closing. /// Return true iff the error was due to a connection closing.
pub(crate) fn connection_closed(&self) -> bool { pub(crate) fn connection_closed(&self) -> bool {
if self.kind() != ErrorKind::Io { if self.kind() != ErrorKind::Io {

View File

@@ -121,7 +121,7 @@ impl Request {
let response = let response =
unit::connect(unit, true, 0, reader, false).map_err(|e| e.url(url.clone()))?; unit::connect(unit, true, 0, reader, false).map_err(|e| e.url(url.clone()))?;
if response.error() && self.error_on_non_2xx { if self.error_on_non_2xx && response.status() >= 400 {
Err(ErrorKind::HTTP.new().url(url.clone()).response(response)) Err(ErrorKind::HTTP.new().url(url.clone()).response(response))
} else { } else {
Ok(response) Ok(response)

View File

@@ -145,30 +145,6 @@ impl Response {
.collect() .collect()
} }
/// Whether the response status is: 200 <= status <= 299
pub fn ok(&self) -> bool {
self.status >= 200 && self.status <= 299
}
pub fn redirect(&self) -> bool {
self.status >= 300 && self.status <= 399
}
/// Whether the response status is: 400 <= status <= 499
pub fn client_error(&self) -> bool {
self.status >= 400 && self.status <= 499
}
/// Whether the response status is: 500 <= status <= 599
pub fn server_error(&self) -> bool {
self.status >= 500 && self.status <= 599
}
/// Whether the response status is: 400 <= status <= 599
pub fn error(&self) -> bool {
self.client_error() || self.server_error()
}
/// The content type part of the "Content-Type" header without /// The content type part of the "Content-Type" header without
/// the charset. /// the charset.
/// ///

View File

@@ -175,7 +175,6 @@ pub fn no_status_text() {
test::make_response(200, "", vec![], vec![]) test::make_response(200, "", vec![], vec![])
}); });
let resp = get("test://host/no_status_text").call().unwrap(); let resp = get("test://host/no_status_text").call().unwrap();
assert!(resp.ok());
assert_eq!(resp.status(), 200); assert_eq!(resp.status(), 200);
} }

View File

@@ -231,7 +231,7 @@ pub(crate) fn connect(
save_cookies(&unit, &resp); save_cookies(&unit, &resp);
// handle redirects // handle redirects
if resp.redirect() && unit.agent.config.redirects > 0 { if resp.status() >= 300 && resp.status() < 400 && unit.agent.config.redirects > 0 {
if redirect_count == unit.agent.config.redirects { if redirect_count == unit.agent.config.redirects {
return Err(ErrorKind::TooManyRedirects.new()); return Err(ErrorKind::TooManyRedirects.new());
} }