Rework Request (and related)

[Fishrock123]

This is essentially a summary of the major improvements to Surf I've been trying to make. there is a lot of overlap with disparate existing issues, which I've linked where appropriate.

---

In order to improve middleware for Surf (#131), and bring it more in-line with Tide, the following changes should be made:

• Middleware should operate on surf::Request, surf::Response rather than http_types::Request, etc.
• surf::Request should be changed to be a more minimalized wrapper around http_types::Request (similar to Response and Tide.)

And also for code hygiene, Request should probably not be all of these at once (#152):

• the sent http structure
• the builder
• the holder of reusable resources (Client, Middleware) (Allow applying middleware before know about request uri and methods #126)
• the consumable Future

Additionally related, Client should not be generic (#69), (comment on 105), and Middleware may want to be accessible from the present Middleware stack for complete retry/redirect purposes (#169), (#18).

Note: Making Client 1:1 with an actualized Request helps this but doesn't work for keep-alive or http2/3+.

---

There is a clean but slightly less ergonomic way to solve most of the API issues here:

• Have surf::Request be like tide::Request, only hold http_types::Request.
• Do not internally hold a surf::Request
• Have Client::send(req: impl Into<surf::Request>) -> BoxedFuture rather than making anything be directly await-able.
  • (Since that requires internal storage and causes the awaited structure to be consumed.)
• Have Middleware be stored on the Client. (Allow applying middleware before know about request uri and methods #126)
• Have a separate RequestBuilder similar to tide::ResponseBuilder.
• Have RequestBuilder::send() be a short-cut for "build and make a new Client and call .send(self) on it".
• Have surf::method() return a RequestBuilder.

let res = surf::post(url)
    .header(h, v)
    .body(b)
    .send().await?;

With middleware:

let req = surf::post(url)
    .header(h, v)
    .body(b);
let client = surf::client()
    .middleware(m);
let res = client.send(req).await?;

Regarding Client being generic, I see two paths. Either #69, or just making the structure's properties be directly determined by compile-time configuration. dyn is probably better if we want people to be able to pass external clients that implement some kind of trait. In that case, surf::client_from(trait) or similar could exist, or possibly even .send::<ClientType>().
Update: see #193 "dyn HttpClient". (Merged)

Ideally I think that Client would be the thing that actualizes/sends Requests and holds the middleware stack, similar to what #109 asks for.

---

This was an older WIP, see #194 for a full compiling patch of the above proposal!

I have a WIP patch of some of this stuff, but in the form of a SurfBuilder, which has a structure like so and is not much cleaner than today, although it does allow middleware to operate on surf::Request and surf::Response. I prefer the approach I described above.

// not great
pub struct SurfBuilder<C: HttpClient + Unpin + Send + Sync> {
    client: C,
    req: Option<Request>,
    middleware: Option<Vec<Arc<dyn Middleware<C>>>>,
    fut: Option<BoxFuture<'static, Result<Response>>>,
}

---

tl;dr things become much cleaner and simpler if:

• Whatever surf::method() returns does not have .middleware() (@yoshuawuyts?) (Allow applying middleware before know about request uri and methods #126)
  • Update: maybe this isn't so bad, so long as it is only transitive to when the Client is made. See RequestBuilder: add .middleware() Fishrock123/surf#1
• Whatever surf::method() returns does not impl Future. (@yoshuawuyts)
  • (I know we discussed this already, but I really think doing this until IntoFuture comes would be better...)
  • Update: This may actually still be ok on RequestBuilder. See src: overhaul Client, rework Request and Middleware, add builder #194
• Client is no longer generic. (@goto-bus-stop?, @yoshuawuyts?) (Making the Client to use dynamic dispatch #69) (Impossible to Create a Static Client? #150)
  • See dyn HttpClient #193 (Merged)

[goto-bus-stop]

great post, i think i agree with this direction. thanks for writing this up!

should we still have surf::post at all if it's just a shorthand for like, surf::RequestBuilder::post()? :)

Have Client::send(req: impl Into<surf::Request>) -> BoxedFuture rather than making anything be directly await-able.

strongly agree with this in particular—it makes the API a bit less "nice" (for some values of nice) but it's better both for the implementation and for actually modeling how requests are sent (client sends requests, requests don't send themselves).

[Fishrock123]

Done in #194.