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

Should send_request_redirect only happen when we know it's a redirect to be followed? #4569

Open
TBBle opened this issue Feb 12, 2020 · 0 comments
Labels

Comments

@TBBle
Copy link
Contributor

TBBle commented Feb 12, 2020

🐣 Is your feature request related to a problem? Please describe.

When implementing the on_request_redirect hook in an integration with a distribute Tracer, I realised I needed to recreate all the logic that follows the send_request_redirect call in order to record enough details to know if it was worth recording the redirect, or if we'd end-up exiting immediately via on_request_exception or on_request_end.

💡 Describe the solution you'd like

If the send_request_redirect call was done around the end of the block, then the details given to on_request_redirect would be the same as would have been given to on_request_start, plus the response to which we are reacting, which gives access to the specific redirect status code, and the incoming headers.

Describe alternatives you've considered

  • Just duplicating the same checks in my hook, to know not to bother logging a redirect annotation if we're not going to call it, and to extract the URL we're being redirected to and the method we're going to use to access it.
  • Just logging that a redirect was called for, without caring where we're going, whether it's devolving to a GET, or whether we'll see any other actions before hitting the "out" code-paths.

📋 Additional context

This would also match the state diagram in the docs (stable and latest are the same) which indicates that "redirect" cannot immediately flow to either "end" or "exception", which is the case now in the case of "redirect without Location/URL header" and "too many redirects" respectively.

@webknjaz webknjaz added the question StackOverflow label Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants