feat(response): preserve URL when converting Response to http::Response
#2798
Conversation
|
If this suggestion is adopted, to be honest, I’m not sure how the ResponseExt API should be designed to align with most people’s usage habits — things like: /// Extension trait for http::Response objects
///
/// Provides methods to extract URL information from HTTP responses
pub trait ResponseExt {
/// Extracts the URL associated with this response
fn url(&self) -> Option<&Url>;
/// Extracts a mutable reference to the URL associated with this response
fn url_mut(&mut self) -> Option<&mut Url>;
/// Takes the URL from the response, consuming it and returning the URL
fn take_url(&mut self) -> Option<Url>;
} |
| /// Extension trait for http::Response objects | ||
| /// | ||
| /// Provides methods to extract URL information from HTTP responses | ||
| pub trait ResponseExt { |
There was a problem hiding this comment.
I'm thinking it might be better to leave this off. Is there a need for it that I'm overlooking?
There was a problem hiding this comment.
I did run into some scenarios where I needed to convert a reqwest::Response into an http::Response while preserving the original URL. That said, as you pointed out, it's always possible to manually clone the URL using the url() method on reqwest::Response. While that approach is a bit more verbose, the only benefit of this change would be to avoid one additional clone, which probably isn't significant in practice.
So I agree with your suggestion—this functionality isn't strictly necessary at the moment. I'm fine with closing this PR, and if a stronger need arises in the future, we can revisit it then.
There was a problem hiding this comment.
Well, I don't mean we need to close the whole thing... I actually have for a while thought we should just store the url in the extensions instead of directly on the struct...
There was a problem hiding this comment.
Alternatively, it's still possible to get the URL with an implementation like this:
impl From<Response> for (Url, http::Response<Body>) {
fn from(r: Response) -> (Url, http::Response<Body>) {
let (parts, body) = r.res.into_parts();
let body = Body::wrap(body);
let response = http::Response::from_parts(parts, body);
(*r.url, response)
}
}
Add URL preservation in the From for http::Response conversion by
inserting the response URL as a ResponseUrl extension. This ensures that the original
URL information is not lost during the conversion and can be accessed later.
This change maintains consistency with the inverse conversion (Fromhttp::Response
for Response) which expects the ResponseUrl extension to be present.