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

wrap reqwest::Response to match Errors #82

Open
peanutbother opened this issue Mar 26, 2023 · 2 comments
Open

wrap reqwest::Response to match Errors #82

peanutbother opened this issue Mar 26, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@peanutbother
Copy link

peanutbother commented Mar 26, 2023

Motivations

[taken from #57]
There is currently no wrapper for reqwest::Response. As a result, resp.body().await? [...]

... returns a reqwest::Error instead of reqwest_middleware::Error.

Solution

It would be possible to create wrapper types but this is not really feasible.
Instead there should be a wrapper to make the api more uniform.

Additional context

I'm implementing reqwest-middleware for https://github.com/peanutbother/api-client to extend its features I'm trying to change to reqwest-middleware which introduces non uniform apis:

https://github.com/peanutbother/api-client/blob/main/src/lib.rs#L453
where

self.request(::reqwest::Method::$method, format!($url).as_str(), $crate::Body::Json(request)).await?.json().await

fails due to the response not being wrapped.

@peanutbother peanutbother added the enhancement New feature or request label Mar 26, 2023
@tl-rodrigo-gryzinski
Copy link
Contributor

tl-rodrigo-gryzinski commented Mar 28, 2023

I might be missing something, if you change your return type to reqwest_middleware::Error, wouldn't it work with reqwest-middleware as-is? You'd need to change it anyways if we were to apply this suggestion here, no?

Also, cool crate 👀

@peanutbother
Copy link
Author

peanutbother commented Mar 29, 2023

the call to response.json().await returns a reqwest::Result instead of a reqwest_middleware::Result

I solved it by mapping the error for now but it would be more convenient if the Result type would be consistent.

I updated the link in the first post as it was pointing to a previous version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants