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

Examples for Error #144

Merged
merged 3 commits into from Jun 11, 2017

Conversation

Projects
None yet
2 participants
@AndyGauge
Copy link
Contributor

AndyGauge commented Jun 9, 2017

Closes #117

@AndyGauge

This comment has been minimized.

Copy link
Contributor Author

AndyGauge commented Jun 9, 2017

In the Issue, @dtolnay asks for examples with depth. Requesting review of example before completing the remaining methods to ensure quality.

@AndyGauge AndyGauge changed the title [WIP] examples for Error Examples for Error Jun 9, 2017

@seanmonstar
Copy link
Owner

seanmonstar left a comment

Thanks for doing this documentation work! I left some comments about how to prevent network operations while running doc tests, and some thoughts on what sort of examples would be typical.

/// fn run() -> Result<Simple, reqwest::Error> {
/// reqwest::get("http://httpbin.org/ip")?.json()
/// }
/// fn main() {

This comment has been minimized.

@seanmonstar

seanmonstar Jun 9, 2017

Owner

It'd be better if the doctests didn't actually make a network request. This can just be # fn main() {}.

///
/// ```
/// # extern crate reqwest;
/// # fn main() {

This comment has been minimized.

@seanmonstar

seanmonstar Jun 9, 2017

Owner

The extern crate and main aren't necessary. This can be:

/// # fn run() {
/// let response = ...
/// # }
/// match response {
/// Err(e) => match e.url() {
/// None => println!("No Url given"),
/// Some(url) => println!("Bad Url given: {}", url),

This comment has been minimized.

@seanmonstar

seanmonstar Jun 9, 2017

Owner

This error probably won't have a URL associated with it, since it wasn't able to parse one. The url() method will have a Url if reqwest was able to parse the error and start an HTTP request. Situations this would have a Url and be useful for:

  • The redirect policy notices a loop. You can check the url() for the last request made that determined there was a loop.
  • There was a network error while making the request. Again, also useful if after a redirect, the server aborted the socket early, while still emitting headers. You could see the io::Error occurred from which server.

This comment has been minimized.

@AndyGauge

AndyGauge Jun 9, 2017

Author Contributor

Difficult to translate into an example. I created a hyper server to give me a redirect loop. I'll post the client example to show the final stop. Unfortunately it wont be reproducible without a redirect loop server (of which I could not find an online example of a redirect loop)

/// let response = reqwest::get(invalid_request);
/// match response {
/// Err(e) => match e.get_ref() {
/// Some(internal_error) => println!("Problem with request: {}", internal_error),

This comment has been minimized.

@seanmonstar

seanmonstar Jun 9, 2017

Owner

I'd guess that @dtolnay meant to show a typical example of why someone would actually use this method. You can already print the reqwest::Error, so why would a user want to call get_ref?

A possible reason could be to try to handle certain cases, like casting to a UrlError and inspecting exactly what was wrong with the provide URL.

@AndyGauge AndyGauge force-pushed the AndyGauge:error-doc branch from f0a1300 to 43c7938 Jun 9, 2017

@AndyGauge AndyGauge force-pushed the AndyGauge:error-doc branch from 43c7938 to 1981fe5 Jun 9, 2017

@seanmonstar seanmonstar merged commit c8d03dd into seanmonstar:master Jun 11, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@seanmonstar

This comment has been minimized.

Copy link
Owner

seanmonstar commented Jun 11, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.