make reqwest and middlewares optional

[matt-phylum]

The client generator emits these dependencies:

reqwest = { version = "0.12", default-features = false, features = ["json", "multipart"] }
reqwest-conditional-middleware = "0.3"
reqwest-middleware = { version = "0.3", features = ["multipart"] }
reqwest-retry = "0.5"
reqwest-tracing = "0.5"

These are causing unnecessary build failures with the latest version of Cargo.

The problem with Cargo

With the latest version of Cargo, cargo update selects reqwest-middleware 0.4 to satisfy a >0.3.0, <0.5.0 dependency from reqwest-tracing. However, the clients generated by this crate specify reqwest-middleware 0.3, so a second copy of reqwest-middleware gets used for that dependency. Because of the way reqwest-middleware and reqwest-middleware work together, they must be using the same version of reqwest-middleware.

error[E0277]: the trait bound `TracingMiddleware<DefaultSpanBackend>: reqwest_middleware::Middleware` is not satisfied
   --> /Users/matt.donoughe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/octorust-0.8.0-rc.1/src/lib.rs:371:19
    |
371 |             .with(reqwest_tracing::TracingMiddleware::default())
    |              ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'a> Fn(reqwest::Request, &'a mut Extensions, Next<'a>)` is not implemented for `TracingMiddleware<DefaultSpanBackend>`, which is required by `TracingMiddleware<DefaultSpanBackend>: reqwest_middleware::Middleware`
    |              |
    |              required by a bound introduced by this call
    |
    = help: the following other types implement trait `reqwest_middleware::Middleware`:
              ConditionalMiddleware<T, C>
              RetryTransientMiddleware<T, R>
    = note: required for `TracingMiddleware<DefaultSpanBackend>` to implement `reqwest_middleware::Middleware`
note: required by a bound in `reqwest_middleware::ClientBuilder::with`
   --> /Users/matt.donoughe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/reqwest-middleware-0.3.3/src/client.rs:51:12
    |
49  |     pub fn with<M>(self, middleware: M) -> Self
    |            ---- required by a bound in this associated function
50  |     where
51  |         M: Middleware,
    |            ^^^^^^^^^^ required by this bound in `ClientBuilder::with`

For more information about this error, try `rustc --explain E0277`.

Optionally used dependencies

reqwest, reqwest-conditional-middleware, reqwest-retry, and reqwest-tracing are only used for Client::new, and should be optional. Client::custom allows creating a working client without them.

reqwest-tracing in particular causes problems because it uses global variables to communicate with the opentelemetry crate and if you're using opentelemetry then you need the versions to match, in addition to matching the version of reqwest-middleware, which is probably why it has a >0.3.0, <0.5.0 dependency instead of a more typical 0.3.1 or 0.4.0.

It should be easy to turn these into optional dependencies with a crate feature. That would solve this dependency problem with reqwest-tracing for Client::custom users.

reqwest versions

octorust 0.8.0-rc.1 depends on reqwest-middleware 0.3, which is outdated. The latest stable version of octorust is even worse, depending on reqwest-middleware 0.2. reqwest-middleware is better than depending directly on reqwest because it allows the client to be customized, but it puts the library on this update treadmill where to continue using Client::custom the dependencies need to be kept up to date with a reqwest-middleware which has breaking API changes more frequently than the API client is updated. I guess it's technically possible to make a reqwest-middleware 0.2 middleware which fulfills the request using a reqwest-middleware 0.4 client (or maybe even a surf client) and use that for making octorust work with a newer reqwest and middlewares, but it's ugly and leaves unused dependencies hanging around.

I think what I'd really like would be if octorust (and the rest) didn't depend on reqwest-middleware or reqwest at all. octocrab uses tower and tower-http for this, so OctocrabBuilder takes something like impl tower::Service<http::Request<impl http_body::Body<Data = bytes::Bytes>>, Response = http::Response<impl http_body::Body<Data = bytes::Bytes>>> + Send + 'static. http is at 1.1 and seems reasonably stable. It's at least much more stable than reqwest-middleware. tower is less mature, but tower::Service is a fairly simple trait version of reqwest-middleware which doesn't expect to ultimately call reqwest at the end.

Unfortunately, it seems like in client scenarios, tower::Service is almost always implemented using hyper_util::client::legacy::Client, but there is a fairly trivial tower-reqwest crate. It's not reqwest-middleware, but it seems straightforward to implement tower::Service for reqwest_middleware::Client to have a seamless transition.

[augustuswm]

Thank you for the thorough writeup! I definitely agree that there are a lot of pain points about the usage of reqwest-middleware in general, and something like this has been on my list of items to work on for a while. I'll take a look at making the middleware portion at least optional as a first step. That should be a fairly tractable change.

I'm not opposed to making the underlying http client generic, but that will take some more thought and time.

[matt-phylum]

The new version supports building without the default middleware, but in that configuration doesn't allow providing custom middleware because it replaces ClientWithMiddleware with Client.