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

feat(redirect): Coverting policy into tower-http policy. #2617

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

linyihai
Copy link

What does this want to do

Working on #2576

Inspired by #2613 (comment)

This PR converts the rewqest::redirect::Policy as tower_http::follow_redirect::policy::Policy.

Why the policy conversion needs happen in the ClientBuild::build function?

Because the Policies from tower-http needs to be mut and will change their inner state during redirect. See the Limited code.

How to test

The first commit add the Limit and Custom policy test. The second commit make the implementation and updated the test.

Others

Needs to do

  • should remove the Policy::check method and test ?
  • Add PR to tower-http

@@ -405,7 +405,7 @@ async fn test_redirect_limit() {
let res = client.get(&url).send().await.unwrap_err();
assert_eq!(
res.url().unwrap().as_str(),
format!("http://{}/redirect/2", server.addr()).as_str()
format!("http://{}/redirect/3", server.addr()).as_str()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior is difference.

@linyihai linyihai changed the title feat(redirect): Using tower-http policy instead. feat(redirect): Coverting policy into tower-http policy. Mar 28, 2025
@linyihai linyihai force-pushed the redirect-refactor branch from b400be6 to 4cf8a0f Compare April 2, 2025 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant