Skip to content

Conversation

@unusual-thoughts
Copy link

This is an intermediate step towards implementing a full python requests-style history

While keeping every intermediate response might be costly, at least knowing if redirects happen and how many is useful to the user, and since we already store the info, why not make it available ?

This simply exposes the urls field from PendingRequest to the Response, and adds a utility function to get a list of all the traversed urls, including the final one.

fixes: #2314

@unusual-thoughts unusual-thoughts force-pushed the expose-urls branch 3 times, most recently from 752a019 to 1a6e8f7 Compare March 24, 2025 06:50
@unusual-thoughts
Copy link
Author

@seanmonstar could you give it a look ?

@unusual-thoughts unusual-thoughts force-pushed the expose-urls branch 2 times, most recently from 89f79f4 to 992140c Compare April 18, 2025 19:59
@seanmonstar
Copy link
Owner

Thanks for the PR! Here's just some thoughts:

  • Would this still be feasible to support with the refactor described in Refactor redirects to use tower-http #2576? I think the answer is yes, but good to be explicit about it.
  • I would consider storing them inside the extensions (with a private History() newtype just to prevent collisions). A non-empty history should be uncommon, so I'd rather not pay the cost of increasing the size of the Response struct.

@unusual-thoughts unusual-thoughts force-pushed the expose-urls branch 2 times, most recently from cbbca05 to 0b7073d Compare April 24, 2025 23:49
@unusual-thoughts
Copy link
Author

Thanks for the PR! Here's just some thoughts:

So I had a look, and currently tower-http's FollowRedirect only stores the final url in an extension, similar to the existing ResponseUrl here that is used for conversions (but confusingly named RequestUri).

It should still be easy to implement incremental history pushes in the tower_http::follow_redirect::Policy wrapping implementation, using tower_http::follow_redirect::policy::Attempt::previous().

  • I would consider storing them inside the extensions (with a private History() newtype just to prevent collisions). A non-empty history should be uncommon, so I'd rather not pay the cost of increasing the size of the Response struct.

Done, now storing the Vec<Url> inside extensions. Note that redirects are actually pretty common and enabled by default, and history will contain at least the original request url if there are any redirects.

@unusual-thoughts
Copy link
Author

@seanmonstar looks good to you ?

@seanmonstar
Copy link
Owner

The other PR re-implementing redirects using tower-http is nearly complete, I'm focused on getting that merged first. And then we can see how to integrate this on top. OK?

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.

Redirect response history

2 participants