Repository: encode/httpx
File: httpx/_client.py (Python)

Code before:
            if not allow_redirects:
                response.call_next = functools.partial(
                    self._send_handling_redirects,
                    request=request,
                    auth=auth,
                    timeout=timeout,
                    allow_redirects=False,
                    history=history,
                )
                return response

    def _send_handling_auth(
        self,
        request: Request,
        history: typing.List[Response],
        auth: Auth,
        timeout: Timeout,
    ) -> Response:
        if auth.requires_request_body:
            request.read()

        auth_flow = auth.auth_flow(request)
        request = next(auth_flow)

        for hook in self._event_hooks["request"]:
            hook(request)

        while True:
            response = self._send_single_request(request, timeout)
            if auth.requires_response_body:
                response.read()
            try:
                next_request = auth_flow.send(response)
            except StopIteration:
                return response
            except BaseException as exc:
                response.close()
                raise exc from None
            else:
                response.history = list(history)
                response.read()
                request = next_request
                history.append(response)

    def _send_single_request(self, request: Request, timeout: Timeout) -> Response:
        """
        Sends a single request, without handling any redirections.
        """
        transport = self._transport_for_url(request.url)

        with map_exceptions(HTTPCORE_EXC_MAP, request=request):

Review comment (security):
Getting my thoughts down here — are we sure this is the correct/best place to call `request` hooks? AFAICS Requests does not provide a `request` hook so we're in green field here.

There are a few options that I can see…

1. In `.send()` or at the beginning of `_send_handling_redirects()` - Hide redirect requests, auth requests, retries requests.
1. At the beginning of `._send_handling_auth()` - Hide auth requests, retries requests.
1. At the beginning of `._send_single_request()` - Hide retries requests.

As I understand it, currently this PR suggests option 2/.

But interestingly the `response` hook is located at level 1/. So in case of a redirect, we'd have `request` hooks called twice or more, but `response` hooks called once.

Is this appropriate, or do we want `request` being called only once (for the initial request) (which corresponds to option 1/)?

Another data point from the monitoring use case perspective — I can see that the Datadog/Requests tracer integration [wraps `requests.Session.send()`](https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/contrib/requests/connection.py), which _I think_ corresponds to case 1/ above.

Perhaps there's a case to be made about having `request`, `redirect_request`, `auth_request`, and `single_request` hooks, or perhaps just `request` and `single_request`, or…? And then, should we match them up with corresponding `response_*` hooks?

I guess for simplicity we'd really want option 1/, i.e. call `request` hooks before calling `._send_handling_redirects()`, and call `response` hooks on the received resolved response?

Code after:
        self,
        request: Request,
        auth: Auth,
        timeout: Timeout,
        allow_redirects: bool,
        history: typing.List[Response],
    ) -> Response:
        auth_flow = auth.sync_auth_flow(request)
        request = next(auth_flow)

        for hook in self._event_hooks["request"]:
            hook(request)

        while True:
            response = self._send_handling_redirects(
                request,
                timeout=timeout,
                allow_redirects=allow_redirects,
                history=history,
            )
            try:
                next_request = auth_flow.send(response)
            except StopIteration:
                return response
            except BaseException as exc:
                response.close()
                raise exc from None
            else:
                response.history = list(history)
                response.read()
                request = next_request
                history.append(response)

    def _send_handling_redirects(
        self,
        request: Request,
        timeout: Timeout,
        allow_redirects: bool,
        history: typing.List[Response],
    ) -> Response:
        while True:
            if len(history) > self.max_redirects:
                raise TooManyRedirects(
                    "Exceeded maximum allowed redirects.", request=request
                )

            response = self._send_single_request(request, timeout)
            response.history = list(history)

            if not response.is_redirect:
                return response