Event hooks

[tomchristie]

Closes #790, based on the design outlined in #790 (comment)

I've narrowed this down to only request and response initially. I think that auth and redirect also make sense, but let's treat that separately. (I want to be careful, since it could be a bit fiddly making sure that raised exceptions within those hooks don't leave responses open.)

Also worth noting that we're not exposing the EventHooks data structure as public API. Event hooks are always set using a regular dictionary, but have smart behaviour when the property is updated, so that they always return lists of installed handlers, but can be set either from a list or from a single callable.

For example:

client = httpx.Client()
assert dict(client.event_hooks) == {'request': [], 'response': []}

client.event_hooks['request'] = some_callable
assert dict(client.event_hooks) == {'request': [some_callable], 'response': []}

client.event_hooks['request'].append(some_other_callable)
assert dict(client.event_hooks) == {'request': [some_callable, some_other_callable], 'response': []}

del client.event_hooks['request']
assert dict(client.event_hooks) == {'request': [], 'response': []}

Rendered docs:

[j178]

💯 👍

[florimondmanca]

I think I'd like us to prefer a stricter typing here, and a more straightforward style that requires using a list of callables:

client = httpx.Client(event_hooks={"request": [on_request], "response": [on_response])

It would make it clearer that client.event_hooks is really just always a dict of lists, rather than "a dict of sometimes a callable, sometimes a list of callables, depending on what you've set" (not even sure that's what this PR intends, actually?).

Hence this suggestion (which would need to be applied across _client.py as well):

Suggested change

	event_hooks: dict = None,
	event_hooks: Dict[str, List[Callable]] = None,

[florimondmanca]

I think if we apply my suggestion of using a plain Dict[str, List[Callable]] always, we can mostly drop this custom data structure, correct?

I'm also not sure we need to enforce that the only possible keys be request and response. Currently we do this with a bit of a surprising behavior ("don't actually store items for unknown keys"), while I think making it so that event_hooks behaves just like a normal dict would be beneficial.

So perhaps we could replace this with a simple factory helper…

def create_event_hooks(initial: Dict[str, List[Callable]] = None) -> Dict[str, List[Callable]]):
    event_hooks = {"request": [], "response": []}
    event_hooks.update(initial or {})
    return event_hooks

[florimondmanca]

What do we think of putting these tests (and those for the sync client) in a dedicated tests/client/test_event_hooks.py file?

[florimondmanca]

I think these changes belong to a separate PR? Or most likely, this PR isn't up to date with master. :-)

[florimondmanca]

Another reason why I'm advocating for a straightforward "dict of lists of callables" is that we wouldn't need any # type: ignore…

Suggested change

	client.event_hooks = {"request": on_request} # type: ignore
	client.event_hooks = {"request": [on_request]}

[florimondmanca]

These tests would be simplified if we adopted a simple create_event_hooks() helper that returns a plain dict.

[Dustyposa]

How can we get the response body in response hook function?
Like this:

async def raise_with_response(cls, res: Response) -> None:
        if res.status_code >= 300:
            # await res.aread(). # if no this code, will occurs an error
            print(res.text). # error occurs: httpx.ResponseNotRead: Attempted to access streaming response content, without having called `read()`.

For read text, I need run await res.aread(). Has any other way todo this?
And I shouldn't do this in event_hooks ? What the best way to handle response with response text?
Thanks in advance.

[tomchristie]

I'm not sure what the question is?

If you want to access the response body inside an event hook, then as you've said, you need to call .read()/.aread(), yes.

[Dustyposa]

I'm not sure what the question is?

If you want to access the response body inside an event hook, then as you've said, you need to call .read()/.aread(), yes.

Thank you, got it.
I just want to know whether should I handle response.text in event_hook or use other way, which one is best way.