Inconsistencies between ureq and reqwest http clients

[andrewhickman]

I was reading through the code and I think there are some small differences in behaviour between ureq::http_client and reqwest::http_client:

• Redirects are disabled for reqwest but are left enabled for ureq
  

  oauth2-rs/src/reqwest.rs

  Line 57 in 30c934a

  .redirect(RedirectPolicy::none())

• By default ureq returns an error for unsuccessful HTTP status codes but reqwest does not

[ramosbugs]

nice catch! I think we should try to make the ureq client match the reqwest client's behavior

[andrewhickman]

I wonder if we should also keep a static reqwest::Client / ureq::Agent to allow for connection reuse. (If my understanding of the curl docs is right, that's effectively what the curl handler does)

[ramosbugs]

I'd prefer not to add any implicit global state, but it should be possible for clients to pass in a closure that references an existing HTTP client without creating a new one on each request. To make this easier, this crate could offer an interface like the following without introducing any breaking changes:

let http_client = oauth2::ureq::new_http_client();
...
client.exchange_code(...).request(http_client.callback())

i.e., new_http_client can return a new UreqHttpClient wrapper struct that holds the client's internal state, and callback can return a closure.

[ramosbugs]

I'm working on some breaking changes for 5.0 (see #253), which will accept reqwest::Client and ureq::Agent as clients instead of using stateless functions. This should bring the two interfaces in line and (somewhat unfortunately) offloads the responsibility to prevent redirects to user code. I'll add a note about this to the docs.

[ramosbugs]

This is now released in 5.0.0-alpha.1. If you can, please try it out and let me know if you find any issues that should be fixed before stabilizing the 5.0 API. There's an Upgrade Guide to help.