follow-redirect: Better integration with user-side redirection handling
tesaguri opened this issue · 0 comments
Feature Request
Motivation
Users may want to implement their own redirection logics like <meta http-equiv="refresh" />
. In that case, they might want to delegate to the FollowRedirect
middleware's Policy
object so that they can reuse the Limited
policy's redirection count for example. But the current FollowRedirect
middleware doesn't provide access to the policy object.
Proposal
Expose a constructor for Attempt
and add a response extension type that contains the policy that was used in the request. With the new interface (let's call the extension type PolicyExtension
for now), the motivating use case can be written like the following:
use http::StatusCode;
use tower_http::follow_redirect;
use tower_http::follow_redirect::policy::{Policy, PolicyExt};
let mut redirect_policy = follow_redirect::policy::Standard::default().or::<_, (), _>(Err(
follow_redirect::policy::redirect_fn(|_| Err(MyError::TooManyRedirects)),
));
loop {
let mut res = FollowRedirect::with_policy(&mut client, policy)
.call(prepare_request(uri))
.await?;
if let ResponseKind::RedirectTo(location) = process_response(&mut res) {
uri = location;
} else {
break;
};
redirect_policy = res
.extensions_mut()
.remove::<follow_redirect::PolicyExtension>()
.unwrap()
.0;
let previous = &res
.extensions()
.get::<follow_redirect::RequestUri>()
.unwrap()
.0;
// This is not quite readable. Maybe this should be a builder instead?
let attempt = follow_redirect::policy::Attempt::new(StatusCode::FOUND, &uri, previous);
assert!(redirect_policy.redirect(&attempt)?.is_follow());
}
A drawback would be that it doesn't let the user access the request body, and so the user should only use it for performing a request with an empty body (even if they can clone the original body, since the body might have been replaced with the BodyRepr::Empty
variant), which I think should be explained in the documantation.
Another drawback would be that exposing a constructor for Attempt
limits the possibility for future API change. If we were going to add an accessor for HeaderMap
for example, we would need to either make it return an Option
or make it return an empty map as a placeholder in the case of user-defined redirections, which might not be very useful for implementers of Policy
.
Alternatives
Add the extension type, but do not expose the constructor for Attempt
With this alternative, the user would still be able to reuse the policy object in the subsequent request, but the user wouldn't be able to inform the policy object of the user-defined redirection.
We might be able to mitigate the pain by adding some inspection methods to the built-in policy types that return the policy's state like the remaining counter of a Limited
, so that the user can, for example, check for the counter by themselves and reconstruct a policy object with a decremented counter, but that still would feel a little awkward. Though inspection methods themselves may be useful regardless of this alternative.
Return the request body too
This might allow some requests with non-empty body to continue, but the user still wouldn't be able to distinguish between BodyRepr::Empty
and BodyRepr::None
, which might be confusing and potentially error-prone.
Do nothing
The user would still be able to implement their redirection logic, resetting the policy object in every user-defined redirection. The user would need to implement their own logic to emulate the policies like Limited
, and that logic wouldn't be able to share data with FollowRedirect
's policy object.