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

Allow customization of internal HTTP client #47

Closed
jtaox opened this issue Mar 3, 2023 · 10 comments
Closed

Allow customization of internal HTTP client #47

jtaox opened this issue Mar 3, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@jtaox
Copy link

jtaox commented Mar 3, 2023

Hello! I'm using your library and I noticed that there's no option to customize the internal HTTP client used for API requests when calling Client::new(). This makes it impossible to implement a proxy for all HTTP requests.

I was wondering if it would be possible to add a customization option for the internal reqwest client, similar to what the chatgpt-api offer. This would allow for greater flexibility and more use cases for this library.

@64bit
Copy link
Owner

64bit commented Mar 4, 2023

Hi @jtaox , happy to know you found it useful.

Can you elaborate more about the use case with custom reqwest/HTTP client? What customization would you like to do?

I havent had to use a proxy yet, so your use case can help me understand better.

@TmLev
Copy link
Contributor

TmLev commented Mar 4, 2023

@64bit sometimes you may have a misbehaving virtual machine that doesn't like certain IP addresses. In such case you can specify an HTTP proxy for reqwest to proxy all the requests.

@jtaox
Copy link
Author

jtaox commented Mar 4, 2023

@64bit In China, some websites are banned by the GFW, making it impossible to access them directly. Therefore, it's necessary to use a foreign server as a proxy to access these websites

@jtaox
Copy link
Author

jtaox commented Mar 4, 2023

@64bit @TmLev Thank you for your patient response. After carefully reviewing the request documentation at proxies, I found that reqwest enables proxies by default, as long as the system environment variables are set correctly. Therefore, #48 does not need to be merged.

@64bit
Copy link
Owner

64bit commented Mar 4, 2023

Aha glad you found a solution :)

Thank you for contributing and sharing about GFW.

I'll close this as it's resolved for you.

@64bit 64bit closed this as completed Mar 4, 2023
@TmLev
Copy link
Contributor

TmLev commented Mar 4, 2023

While proxies may be set up by the means of environment variables, supporting custom middleware is still a thing :)

For instance, there is a tracing middleware which enables the use of OpenTelemetry traces: https://crates.io/crates/reqwest-tracing

@64bit
Copy link
Owner

64bit commented Mar 4, 2023

Hi @TmLev , Your use case is different from the original need for it, but this issue is broad enough so I'll re-open this.

For having middlewares on client side, I see more info on seanmonstar/reqwest#155 and https://truelayer.com/blog/adding-middleware-support-to-rust-reqwest/

So until reqwest natively support it, PRs are welcome, if this changes the type signature then additional work required in #48?

@64bit 64bit reopened this Mar 4, 2023
@64bit 64bit added the enhancement New feature or request label Mar 5, 2023
@TmLev
Copy link
Contributor

TmLev commented Mar 13, 2023

I doubt reqwest will support it natively in the near future, which makes reqwest-middleware the only alternative (the other one is using tower::Service which is quite cumbersome).

Unfortunately, reqwest-eventsource only works with reqwest::RequestBuilder, so I'll need some time to glue them together.

In the meantime, I did a small optimization (which was a low-hanging fruit, really): #58

@64bit
Copy link
Owner

64bit commented Mar 15, 2023

Nice optimization!

tower::Service indeed cumbersome but most powerful if it can be leveraged client side.

reqwest-middleware would be a good step. Please take your time, no rush.

@64bit
Copy link
Owner

64bit commented Mar 4, 2024

Happy 1 year anniversary for this issue 😄

I think its safe to close it, seems like having feature to provide reqwest client in with_http_client is good enough.
Hence, closing this.

@64bit 64bit closed this as completed Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants