litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.53k stars 377 forks source link

Enhancement: Allow multiple lifecycle hooks of each type to be run on any given request #3752

Open charles-dyfis-net opened 1 month ago

charles-dyfis-net commented 1 month ago

Summary

Only having one lifecycle hook per request makes it impossible to use multiple plugins which both set the same hook, or makes it easy to inadvertently break plugins by overriding a hook that they install and depend on (whether at the root application or a lower layer).

It also places in application code the responsibility of being aware of hooks installed by outer layers, and ensuring that any hooks at inner layers take responsibility for those outer hooks' functionality.

The simplest approach to addressing this is to allow each layer to define a list of hooks, and run all of them for each request (halting when any hook reaches a terminal state, for those hooks where this applies).

Basic Example

Just as guards are defined as a list (where this list is merged between layers to assemble a final list of guards), under this proposal the same would be true of lifecycle hooks.

Drawbacks and Impact

This is a breaking change from an API perspective: Documentation and test cases would need to be updated, plugins or other code assigning lifecycle hooks would need to be modified.

Unresolved questions

Compared to the proposal embodied in #3748, this provides less flexibility -- hooks following that proposal can decide to invoke their parents either before or after themselves, or can modify results returned by parent hooks if they so choose.

However, with this reduction in flexibility there is also a substantive reduction in responsibility: using that proposal, a hook could inadvertently prevent other hooks from running, most concerningly by way of not simply implementing the newer interface.


[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

provinzkraut commented 1 month ago

How about this:

charles-dyfis-net commented 1 month ago

How about this:

  • before_request: (request: Request, response: Response | None) -> Response | None. Callables will be reduced. If one hook creates a Response, the following hooks will be able to modify it
  • after_request: (response: Response) -> Response: Same semantics as before_request: Callables will be reduced
  • after_response: (response: Response) -> None: Callables will be mapped and cannot influence each other.

This is all reasonable. If I were designing this myself I might have called the first before_request returning a response terminal rather than invoking the others (even though that would have made behavior exercised in the test suite for #3748 impossible), but that's a place where there are multiple reasonable options -- it's a choice between flexibility (but more room for error) vs deliberate inflexibility (to provide an interface that's hard to misuse), and I think either direction is both workable and defensible.