http-rs / surf

Fast and friendly HTTP client framework for async Rust
https://docs.rs/surf
Apache License 2.0
1.46k stars 120 forks source link

Different approach to "per-request" middleware #205

Closed goto-bus-stop closed 4 years ago

goto-bus-stop commented 4 years ago

This is an idea assuming that #194 or a similar approach will land. That PR will likely remove per-request middleware.

A different approach could be to have multiple clients sharing the same client backend, but different middleware stacks. For example,

let base_client = surf::Client::new()
  .middleware(Logger::new())
  .middleware(AuthenticationMiddleware::new());
let retriable_client = base_client.clone()
  .middleware(Retry::new());

// Now use retriable_client.send() for things that you know can be retried, and base_client for other things

In this snippet, client.clone() would clone the middleware stack and clone the Arc<dyn HttpClient>. The Arc-ed resources would be shared between clients, while non-Arc-ed resources are specific to a client.

If surf gets connection pooling support in the future, I would expect that retriable_client and base_client share the same connection pool, but base_client has 2 middlewares and retriable_client has 3. Other configuration knobs like timeouts or proxying could be treated in the same way as middlewares (if they are not implemented as middlewares).

If a particular middleware should really only be used for a single request, that could be achieved by creating a temporary client clone with

base_client.clone()
  .middleware(SomethingUseful::new())
  .send(req)

This is similar to the approach taken by okhttp in Java land, where you can add interceptors to a client (functionally identical to middleware) and use client.newBuilder() to create new client instances based on existing instances while sharing the backing thread pool.

Fishrock123 commented 4 years ago

Something similar was also brought up by ReAnna#9061 (edit turns out that's the op) in the surf discord - i.e. that Client::clone() should copy the middleware stack but not share it like an Rc.

(Fwiw, tide::Server has this same issue.)