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

Event hooks do not allow for mutation of original response #1343

Closed
2 tasks done
coltoneakins opened this issue Oct 3, 2020 · 3 comments
Closed
2 tasks done

Event hooks do not allow for mutation of original response #1343

coltoneakins opened this issue Oct 3, 2020 · 3 comments
Labels
question Further information is requested requests-compat Issues related to Requests backwards compatibility

Comments

@coltoneakins
Copy link
Contributor

coltoneakins commented Oct 3, 2020

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

Event hooks do not facilitate mutating the original response/request for which the hook is attached to. The problem is two-fold:

  1. Properties of response instances do not have setters to mutate properties like response.text. Raises "AttributeError: can't set attribute".
  2. In httpx, event hooks do not return a response/request. Hooks are just passed the response/request as a parameter. This is contrary to way Requests handles event hooks. This makes it difficult to mutate (or completely replace) the response/request.

(I realize this may be 'by-design' to avoid side-effects--like mutations (i.e. to make programming with httpx more 'functional'). But, I am submitting this bug anyway.)

To reproduce

The point of this code sample is to set up something like 'retries' or a 'HTTP interceptor'. If we get a 401 for any request made using the httpx client, try logging in again and re-send the original request.

However for the sake of this bug report, I made this retry whenever a 404 response was received from the API. Just to illustrate the bug.

  1. Set up an environment with pipenv and install httpx.

  2. Use this code sample:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-


###########
# IMPORTS #
###########

import httpx
import pdb

#########
# HOOKS #
#########

# This is a factory funciton that returns the actual hook that will be used
def try_again(client):
    # This function uses a closure for the retry_counter
    # This is so the function try_again_hook can 'remember' the retry_counter each time it's called.
    retry_counter = 0

    def try_again_hook(response):
        nonlocal retry_counter # because of the closure

        if retry_counter > 3: # try a max of 3 times
            # We tried loggin in too many times and this request still doesn't work. So, raise an error.
            raise Exception("Too many tries!")
        
        if response.status_code == 404:
            retry_counter += 1 # increment the retry counter

            new_response = client.get("https://pokeapi.co/api/v2/pokemon/charmander") # not a 404

            #############
            # BUG START #
            #############

            # Ok, lets try just mutating the original response:
            #response.text = "test" # DOES NOT WORK -- Attribute Error. The 'text' property is set using the @property decorator
            # Ok, lets try mutating a different property:
            #response.status_code # DOES WORK -- can manipulate instance variable

            # Ok, lets try replacing the response:
            #response = new_response # DOES NOT WORK -- Original 404 response is returned after the hook is done
            # Parameters are local in Python, cannot replace the original response.

            # Ok, lets try returning a response:
            return new_response # DOES NOT WORK -- Httpx is not coded for event hooks to return values

            ###########
            # BUG END #
            ###########
        
        else:
            # essentially if no 404, do nothing to the response
            retry_counter = 0 # reset the retry counter if we didn't get a 404

        
    return try_again_hook


########
# MAIN #
########

def main():
    # Create a requests client to send the headers with all requests
    c = httpx.Client()
    # Set up an event hook
    c.event_hooks["response"].append(try_again(c))
    # Set up retries for requests in case a request fails
    test = c.get("https://pokeapi.co/api/v2/pokemon/test") # this will 404 unless hook replaces response
    print(test.text)
    
main()

Expected behavior

The original response to the 404 resource is replaced with a 200 response to another resource--because that is how the hook is designed to work (i.e. with the ability to mutate the original response or substitute another one).

Actual behavior

Hooks cannot mutate nor replace the original response object given to them. In the code sample above, not all properties of the Response instance can be mutated. Also, the Response instance cannot be replaced because event hooks in httpx do not return a value (unlike in Requests for Python).

Debugging material

Code gives

Not found

despite attempting to return a 200 resource response.

Environment

  • OS: Linux
  • Python version: Python 3.6.9
  • HTTPX version: 0.15.5
  • Async environment: N/A, code is not async
  • HTTP proxy: no
  • Custom certificates: no

Additional context

  • Event hooks in Requests return the response/request: See this link
  • This allows hooks in Requests to mutate the response/request. For example, to handle 401 codes as described in this StackOverflow answer.

How to Fix an Issue Like This

  • Add appropriate setters to all properties on response and request instances. For example, response.text found here in the httpx source code: link
  • Refactor how event hooks work to return response/request instances similar to how Requests uses event hooks. The way httpx event hooks currently work makes it difficult to mutate (or completely replace) the original response/request.
@johtso
Copy link
Contributor

johtso commented Oct 4, 2020

You might want to check out custom transports which give you this kind of control https://www.python-httpx.org/advanced/#custom-transports

Also see the discussion regarding adding Middleware functionality #345

@florimondmanca florimondmanca added question Further information is requested requests-compat Issues related to Requests backwards compatibility labels Oct 4, 2020
@florimondmanca
Copy link
Member

florimondmanca commented Oct 4, 2020

@coltoneakins Hi,

Yup this aspect is definitely by design. Event hooks are meant for tapping into requests and responses as they flow through an application, but they're not meant to change that flow in any meaningful way. If you'd like to gain that kind of control, then I'd suggest you look at resources @johtso linked to. I'll admit we're aware there's some ironing out required from a UX perspective, but we're slowly getting there and I hope we'll soon be able to provide some more compelling examples of how custom transports can be used to alter / enhance request flows.

Closing for now; if you're running into issues feel free to reach back, or head to the chat so we can discuss anything you'd see as a blocker. :)

@florimondmanca
Copy link
Member

Oh and actually, if this isn't something we have in our Requests compatibility guide already, I assume we'd gladly accept a PR adding there that event hooks aren't designed to allow mutating responses or performing extra requests, linking to our custom transports docs as the best alternative.

florimondmanca added a commit that referenced this issue Dec 23, 2020
* Updates compatibility guide to address event hooks

In `requests`, event hook callbacks can mutate response/request objects. In HTTPX, this is not the case.
Added text to address this difference, and added a link to the best alternate HTTPX offers in this circumstance.
 
More context:
#1343 (comment)

* Apply suggestions from code review

Co-authored-by: Florimond Manca <[email protected]>

Co-authored-by: Florimond Manca <[email protected]>
WorldStar0126 pushed a commit to WorldStar0126/httpx that referenced this issue Mar 30, 2023
* Updates compatibility guide to address event hooks

In `requests`, event hook callbacks can mutate response/request objects. In HTTPX, this is not the case.
Added text to address this difference, and added a link to the best alternate HTTPX offers in this circumstance.
 
More context:
encode/httpx#1343 (comment)

* Apply suggestions from code review

Co-authored-by: Florimond Manca <[email protected]>

Co-authored-by: Florimond Manca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested requests-compat Issues related to Requests backwards compatibility
Projects
None yet
Development

No branches or pull requests

3 participants