- Don't panic on the mutex in all tests if a single test fails
- Give a more helpful message if a test handler wasn't registered
- Enable env_logger for tests
Status code must be exactly three digits.
HTTP version must be "HTTP/", digit, dot, digit.
https://tools.ietf.org/html/rfc7230#section-3.1.2
status-line = HTTP-version SP status-code SP reason-phrase CRLF
After the recent changes in #257, it's probably not necessary. It's now
quite easy to use a match statement to extract responses for certain
status codes, or all status codes.
Add documentation on how to turn a status code error back into a
Response.
This allows Error to report both the URL that caused an error, and the
original URL that was requested.
Change unit::connect to use the Response history for tracking number of
redirects, instead of passing the count as a separate parameter.
Incidentally, move handling of the `stream` fully inside `Response`.
Instead of `do_from_read` + `set_stream`, we now have `do_from_stream`,
which takes ownership of the stream and keeps it. We also have
`do_from_request`, which does all of `do_from_stream`, but also sets the
`previous` field.
We should include the `fn main() -> Result` part, because people may not
know they need the `-> Result` in order for `?` to work.
Also, I replaced `Accept: text/html` with `Example-Header: header
value`. My thinking is that if someone doesn't know they need to remove
that Accept for non-HTML URLs, they might get unexpected results.
After the recent changes in #257, it's probably not necessary. It's now
quite easy to use a match statement to extract responses for certain
status codes, or all status codes.
Change "Bad" to "Invalid" in error names, mimicking io::Error::ErrorKind.
Change InvalidProxyCreds to ProxyUnauthorized.
Change DnsFailed to just Dns (the fact that there was a failure is implicit
in the fact that this was an error).
Now that Responses with non-2xx statuses get turned into `Error`,
there is less need for these. Also, surveying the set of public crates
that depend on ureq, none of them use these methods. It seems that
users tend to prefer checking the status code directly.
Here is my thinking on each of these individually:
.ok() -- With the new Result API, any Request you get back will be
.ok(). Also, I think the name .ok() is a little confusing with
Result::ok().
.error() - with the new Result API, this is an exact overlap with
anything that would return Error. People will just check for whether a
Result is Err(...) rather than call .error().
.client_error() - most of the time, if someone wants to specially handle
a 4xx error, they want to handle specific ones, because the response to
them is different. For instance a specialized response to a 404 would be
"delete this from the list of URLs to check in the future," where a
specialized response to a 401 would be "try and load updated
credentials." For instance:
https://github.com/msfjarvis/healthchecks-rs/blob/4200edb9ed73bed56a05d81cdd3b0a65badc7ede/healthchecks/src/manage.rs#L70-L84https://github.com/SirWindfield/cargo-free/blob/75d4b363b63fa2b6f3ac12a196a2cb80f0afc848/src/lib.rs#L59-L63https://github.com/lukehsiao/netlify-ddns-rs/blob/1d7daea38bf78d9f1bbfba9d38ea1863ed70ed1d/src/netlify.rs#L101-L112
.server_error() - I don't have as much objection to this one, since it's
reasonable to want to treat all server errors (500, 502, 503) more or
less the same. Although even at that, 501 Not Implemented seems like
people would want to handle it differently. I guess that doesn't come up
much in practice - I've never seen a 501 in the wild.
.redirect() - Usually redirects are handled under the hood, unless
someone disables automatic redirect handling. I'm not terribly opposed
to this one, but given that no-one's using it and it's just as easy to
do 300..399.contains(resp.status()), I'm mildly inclined towards
deletion.
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 moves Stream's enum into an `Inner` enum, and wraps a single
BufReader around the whole thing. This makes it easier to consistently
treat the contents of Stream as being wrapped in a BufReader.
Also, implement BufRead for DeadlineStream. This means when a timeout
is set, we don't have to set that timeout on the socket with every
small read, just when we fill up the buffer. This reduces the number
of syscalls.
Remove the `Cursor` variant from Stream. It was strictly less powerful
than that `Test` variant, so I've replaced the handful of Cursor uses
with `Test`. Because some of those cases weren't test, I removed the
`#[cfg(test)]` param on the `Test` variant.
Now that all inputs to `do_from_read` are `impl BufRead`, add that
as a type constraint. Change `read_next_line` to take advantage of
`BufRead::read_line`, which may be somewhat faster (though I haven't
benchmarked).
Martin Algesten