redirect and error reform

[seanmonstar]

• Error has been made an opaque struct.
• RedirectPolicy now makes use of RedirectAttempt and RedirectAction.

[Arnavion]

The problem with hiding the underlying error behind std::error::Error::cause() is that it's no longer possible to actually get to said underlying error and do something based on what kind of error it is.

With v0.5 I could have code that, say, retried the request on ConnectionAborted:

match send(url) {
    Err(::reqwest::Error::Http(::reqwest::HyperError::Io(ref io_err))) => match io_err.kind() {
        ::std::io::ErrorKind::ConnectionAborted => /* retry */,
        _ => /* abort */,
    },
}

but with v0.6 the result of reqwest_err.cause().unwrap() cannot be downcast to a concrete type.

Would it make sense to add as_http(&self) -> &::hyper::Error, as_serialization(&self) -> &::serde_urlencoded::ser::Error, etc methods to Error ?

[seanmonstar]

I believe the cause can be downcast. This doesn't work?

if let Some(cause) = err.cause() {
    match cause.downcast_ref::<hyper::Error>() {
        Some(hyper::Error::Io(ref io)) => assert_eq!(io.kind(), ErrorKind::ConnectionAborted),
        _ => ()
    }
}

The ::<hyper::Error> may not even be needed, since it could be inferred by the match, I think.

[Arnavion]

std::error::Error::downcast_ref is similar to Any::downcast_ref and thus requires a 'static bound. It's a long-standing issue - rust-lang/rust#35943

[seanmonstar]

Oh what the...

[seanmonstar]

Ok, so there could be a few solutions to this:

• Add an inherent (so not impl Trait for Error) fn cause(&self) -> Option<&Error + 'static>, so that downcasting can work.
• Add some TryFrom impls to allow static downcasting. Not stable yet, but will be stable in 1.18.
• Add as_http() -> Option<&hyper::Error> and friends, as suggested.
• Add some sort of is_io_kind(&self, std::io::ErrorKind) -> bool method.
• Something else?

[compressed]

I was bitten by this too in 0.6, as I have code that does retries on ConnectionAborted. The first option seems to be the most straight-forward and flexible, although it could be confusing having two versions of the cause fn. The is_io_kind is an interesting option, that would work for me, but I guess if others are matching on other Error variants it won't work for them.

[seanmonstar]

I'm still not certain what would be the best thing to expose.

Tangentially, would there be benefit in reqwest retrying the requests itself, when the method is idempotent?

[seanmonstar]

Oh, looking at std::io::Error, it has an inherent get_ref method that was added since cause wasn't usable. Perhaps reqwest::Error should get get_ref?

[Arnavion]

get_ref is a good idea.

[seanmonstar]

I just published v0.6.1 that adds Error::get_ref that allows downcasting.

[Arnavion]

Works like a charm. Thanks!