Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Request independently from the client? #385

Open
yageek opened this issue Nov 8, 2018 · 10 comments
Open

Create Request independently from the client? #385

yageek opened this issue Nov 8, 2018 · 10 comments
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.

Comments

@yageek
Copy link
Contributor

yageek commented Nov 8, 2018

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
Copy link
Owner

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

@yageek
Copy link
Contributor Author

yageek commented Nov 9, 2018

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>)
  1. 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
Copy link

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

@seanmonstar seanmonstar added the B-rfc Blocked: Request for comments. More discussion would help move this along. label Jan 3, 2019
@sashaweiss
Copy link

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
Copy link

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
Copy link

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
Copy link

bbaldino commented Apr 28, 2023

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
Copy link

bbaldino commented Apr 28, 2023

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

@florianepitech
Copy link

I will be happy if this implementation is valided !

@joshka
Copy link

joshka commented Sep 21, 2024

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 hyperium/http#183 as well as a fairly detailed background on extensions in hyperium/http#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: Request for comments. More discussion would help move this along.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants