aio-libs / aio-libs/aiohttp

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

Open
#4,569 0 comments 0 reactions 0 assignees View on GitHub
enhancement question
Dominant language
Python
Stars
16.5k
Forks
2.4k
Avg merge
17h 22m
Merged PRs (30d)
212

Description

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

When implementing the [`on_request_redirect`](https://aiohttp.readthedocs.io/en/stable/tracing_reference.html#aiohttp.TraceConfig.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](https://github.com/aio-libs/aiohttp/blob/bb7a4eaf21782612cc4c24fa75b9368ddb204cc7/aiohttp/client.py#L489) 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`](https://aiohttp.readthedocs.io/en/stable/tracing_reference.html#aiohttp.TraceConfig.on_request_exception) or [`on_request_end`](https://aiohttp.readthedocs.io/en/stable/tracing_reference.html#aiohttp.TraceConfig.on_request_end).

💡 **Describe the solution you'd like**

If the `send_request_redirect` call was done around the [end of the block](https://github.com/aio-libs/aiohttp/blob/bb7a4eaf21782612cc4c24fa75b9368ddb204cc7/aiohttp/client.py#L545), 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](https://aiohttp.readthedocs.io/en/stable/tracing_reference.html#overview) and [latest](https://aiohttp.readthedocs.io/en/latest/tracing_reference.html#overview) 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.