Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it easier to tell if an error is due to timeout #229

Closed
KodrAus opened this issue Dec 11, 2017 · 3 comments
Closed

Make it easier to tell if an error is due to timeout #229

KodrAus opened this issue Dec 11, 2017 · 3 comments
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.

Comments

@KodrAus
Copy link
Contributor

KodrAus commented Dec 11, 2017

Right now we bury the timeout error in an io::Error with a different inner kind depending on the platform. It would be nice to be able to tell whether an error is from a timeout (so we could retry/reschedule the request) without having to know that it's captured in an io::Error variant.

What do you think about adding an is_timeout method to reqwest::Error to do this? We could either look for any captured io::ErrorKind::WouldBlock/io::ErrorKind::TimedOut and return true. Or we could add an extra variant to reqwest::error::Kind so only errors from Waited are considered timeouts.

@seanmonstar
Copy link
Owner

Seems like a reasonable desire. I remember raising an issue on the Rust repo about how it would be nice to consolidate a timeout error into a single ErrorKind, but back then it was declared better to report exactly what the OS declared.

With that in mind, I'm not certain (as in I'm neutral, not negative) if we should collect these errors into a higher reqwest Kind or not. I also have a tiny worry that translating a WouldBlock into a timeout without thought could have misleading errors when using async...

@seanmonstar seanmonstar added the B-rfc Blocked: Request for comments. More discussion would help move this along. label Dec 11, 2017
@KodrAus
Copy link
Contributor Author

KodrAus commented Dec 11, 2017

Yeh, I was wondering about the async + WouldBlock case too... I think it would probably, usually, approximately be about right to treat WouldBlock in the reqwest::Error as a timeout, since in the futures machinery we communicate readiness through Async, or through the io::Error rather than through reqwest::Error. So if I get a hold of a reqwest::Error I'd assume whatever future produced it is probably dead and can't be polled again.

@seanmonstar
Copy link
Owner

There's now a Error::is_timeout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.
Projects
None yet
Development

No branches or pull requests

2 participants