seanmonstar / reqwest

An easy and powerful Rust HTTP Client
https://docs.rs/reqwest
Apache License 2.0
9.78k stars 1.1k forks source link

Create Request independently from the client? #385

Open yageek opened 5 years ago

yageek commented 5 years ago

I would like to create and modify a Request object independently from a Client. Actually, the API kind of enforce the creation of a RequestBuilder through a Client instance.

Is there any reason to this? Is it planned to update the API of RequestBuilder to offer a way to construct Request like:


let req = Request::builder()
               .header("User-Agent", "Some-Agent")
               .method(Method::Get).body(())
               .timeout(Duration::from_sec(10))
               .build();
seanmonstar commented 5 years ago

It's possible to make and use a Request without a Client, and then eventually call client.execute(req).

yageek commented 5 years ago

I read my first comment and I realised that I was not really clear.

I would like to be able to create a Request whose components would be provided little by little:

// API Host and method are known here
let req_uri = reqwest::Url::parse("https://www.some.com/api/").unwrap();
let mut req = reqwest::Request::new(reqwest::Method::POST, req_uri);

// A test is done to determine if a body needs to be attached
if let Some(body) = potential_body() {
   let bytes = serde_json::ser::to_vec(&body).unwrap();
   *req.body_mut() = Some(reqwest::Body::from(bytes));
 };

 if is_protected() {
      req.headers_mut()
               .insert("Header", "Some Value".parse().unwrap());
 }

It is doable but playing with mutable references does not sound really nice.

A RequestBuilder struct exists. As its name imply, I expect it to be able to use it to create my Request object as proposed in my first comment.

Currently, there is two ways to obtain one RequestBuilder instance:

  1. Using the non documented RequestBuilder::new method:
    RequestBuilder::new(client: Client, request: ::Result<Request>)
  2. Using the Client::request or one of its derived methods:
    pub fn request<U: IntoUrl>(&self, method: Method, url: U) -> RequestBuilder

I would expect to be able to create a instance of the builder without requiring any dependencies on Client or Request:

// Or RequestBuilder::new()
let req = Request::builder()
               .method(Method::Get)
               .uri("http://some.com/api")
               .header("User-Agent", "Some-Agent")
               .timeout(Duration::from_sec(10))
               .body(()) // Or .body((())).build() 
unleashed commented 5 years ago

Related: would it be possible to directly support the types in the http crate so as to consume Requests created using that crate?

sashaweiss commented 5 years ago

I'd also be interested in something like this, or another pattern for "specify everything I need to know to execute a request" that I can then hand to a Client to be executed and handled in some uniform way. Is this something y'all would be open to, or are there reasons to keep RequestBuilder instantiation via a Client?

camsteffen commented 3 years ago

I've been thinking about this and I have a potential solution.

Problem

There are currently two ways to build and send a request:

  1. The "right" way:
    client.get()
      // customize...
      .send();

    Pros: Builder pattern. Yes please! Cons: I have to start building the request from scratch. There is no notion of a standalone "request template" without a Client.

  2. The "other" way:
    let mut request = Request::new(Method::GET, url);
    request.headers_mut().insert(...);
    request.body_mut() = ...;
    client.execute(&request);

    Pros: I can build a Request "template" without Client. That is, I can Request::try_clone and modify for individual requests. That way, my request defaults are not tied to the Client (see ClientBuilder::default_headers). Cons: Ugly. I can't do request.to_builder(). I can't use any shared logic that uses RequestBuilder.

The docs for Client::execute state

You should prefer to use the RequestBuilder and RequestBuilder::send().

This is understandable when weighing the two options above. But it's unfortunate that I can't have the best of both worlds.

I think the crux of the problem is that RequestBuilder contains a reference to Client.

My suggestion

Goals

  1. Use RequestBuilder without Client
  2. Request::to_builder
  3. One way to do things
  4. Deprecations without breaking (if so desired)

Changes

To be added

  1. Put an Optional here: RequestBuilder { client: Optional<Client> }. New functionality described below will not use that field.
  2. Add Request::<httpmethod>() -> RequestBuilder methods and Request::builder() (defaults to GET).
  3. Add Request::to_builder.
  4. Encourage usage of Client::execute.
  5. Make RequestBuilder::send work without a Client when it is not available, similar to the get convenience function. This would have not affect on existing code and avoids panicing on new code.

To be deprecated or removed

  1. Deprecate Client::<httpmethod>() -> RequestBuilder
  2. Deprecate RequestBuilder::send. Or maybe not? It could be left in as a convenience to avoid building a Client.
  3. Consider renaming Client::execute to Client::send. (purely aesthetic)
  4. Consider deprecating Request::new and all Request::*_mut methods, making the struct immutable and encouraging use of RequestBuilder.

Unfortunately this is quite invasive. But I think it is worth it as it creates a more flexible and consistent API.

Note: I am taking some inspiration from Java's java.net.http package (for example see HttpRequest). This was introduced with Java 11 (so it's kinda new) and I think it is a good comparison, very similar goals. A further consideration from that API is the way the request builder accepts the body with the http method such as HttpRequest.Builder#POST(body).

QDoussot commented 3 years ago

I have the same need. I have a function returning a Request object. That would be more comfortable to be able to build a request through the RequestBuilder (I am using the bearer_auth function) without actually using the client. So I can remove the client as a parameter of the function.

It's possible to make and use a Request without a Client, and then eventually call client.execute(req).

Would it be ok as a workarround to do let my_req = Client::new().get(url).bearer_auth(token)....; and later on: my_client.execute(my_req);

Or is there any issue doing that ?

bbaldino commented 1 year ago

It'd be really nice to have all the conveniences in RequestBuilder (basic_auth, forms, etc.) which don't seem to require a client, available when constructing a Request without a client.

A use case (which may be misguided, I'm fairly new to all this): I'm trying to implement some oauth middleware which will grab a new token (using a refresh token) when a 201 is received after sending a request. This means I need to craft a new request in middleware, where I don't have a client.

Would one possible way forward be to:

Create a new RequestBuilder type that didn't require a client. The existing RequestBuilder could leverage that builder for building its Request internally to avoid duplicating the logic. Then nothing breaks: we just have a new RequestBuilder that doesn't require a client. The hardest part is coming up with a name for that new RequestBuilder type :)

bbaldino commented 1 year ago

@seanmonstar I took a swing at what this might look like here

florianepitech commented 1 year ago

I will be happy if this implementation is valided !

joshka commented 1 week ago

This can be implemented without any changes to reqwest by using http::Request::builder() and adding some extension trait methods. This can be done in your app, or possibly would be worth doing in reqwest itself.

To support any parts of the reqwest builder that aren't on the http crate (e.g. the timeout field), add them as extensions, and provide a custom conversion.

E.g. this compiles:

use std::time::Duration;

use http::Method;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let http_request = http::Request::builder()
        .header("User-Agent", "Some-Agent")
        .method(Method::GET)
        .timeout(Duration::from_secs(10))
        .body("")?
        .try_into_reqwest_response()?;

    Ok(())
}

trait RequestBuilderExt {
    fn timeout(self, timeout: Duration) -> Self;
}

impl RequestBuilderExt for http::request::Builder {
    fn timeout(self, timeout: Duration) -> Self {
        self.extension(TimeoutExtension(timeout))
    }
}

#[derive(Debug, Clone)]
struct TimeoutExtension(Duration);

trait TryIntoReqwestResponse {
    fn try_into_reqwest_response(self) -> Result<reqwest::Request, reqwest::Error>;
}

impl<T: Into<reqwest::Body>> TryIntoReqwestResponse for http::Request<T> {
    fn try_into_reqwest_response(self) -> Result<reqwest::Request, reqwest::Error> {
        let timeout = self.extensions().get::<TimeoutExtension>().cloned();
        let mut reqwest_request = reqwest::Request::try_from(self)?;
        if let Some(timeout) = timeout {
            *reqwest_request.timeout_mut() = Some(timeout.0);
        }
        Ok(reqwest_request)
    }
}

There's some prior suggestions that this was going to be done at https://github.com/hyperium/http/issues/183 as well as a fairly detailed background on extensions in https://github.com/hyperium/http/issues/395

I suspect that it would be a good idea for reqwest to add an extension trait with methods like timeout above on http::Request, and move the code that pulls them out into the TryInto<reqwest::Request> implementation rather than a secondary type.