Latest event hook improvements


#1

The latest event hook behavior brings 'em pretty much into line with what you’d expect from middleware.

  • on_response and on_error now run in the reverse order.
  • Event hooks are instantiated on the start of each request/response cycle, and can store state.

The recommended way to pass them is now as a list of classes, however the old-style pass-as-instances will continue to work until the 0.6 version bump.

Should make writing middleware look pretty much as you’d expect it too now.

I expect there may be a few remaining refinements around exactly what happens when some response event hook raises an exception, but I think we’re pretty much there.


#2

It is good to have events tied to a request, but what about framework live cycle events. For example how can I initialize a database once event loop is available, but before going into run_forever?

And if I want to do some initialization at startup, but once database is available?


#3

You should be able to handle those sorts of things in the app.py module directly.

(Tho we might choose to add on_startup hooks at some point)


#4

Why event hooks are all executed always? This makes impossible to make some basic hooks like CORS properly, in Django middleware if a middleware returns a response it stops there. Now the only way to cut the chain I think is by raising an exception, which is fine for some use cases, but not for others that are not errors.

e.g if you try to accomplish CORS as a hook, you found yourself that you cannot stop the hooks chain with returning a response in case the method is OPTIONS, which would be perfect. Now it continues and raises a MethodNotAllowed exception no matter what, so no way to make this easy for API’s that need pre-flight cors, as you would to duplicate the routes with OPTIONS? no way. Maybe there is something not documented or that I did not reach at the docs, but looking at the code it seems pretty clear the Injector will execute them all.


#5

From what I have seen in the source code, the only way you can prevent event hooks from firing is using the standalone=True param on the Route. It discards all the handlers though. If you really want to change this behavior you could implement a subclass of the App and override the __call__.

So the Injector is probably not the problem.


#6

Looks like it’s here not in call where it runs all the hooks

In Injector.run/run_async is where the hooks are run, and they are all sequentially executed depending on get_even_hooks order. If the execution stopped when a request or response handler returned a Response object, it would be as powerful as Django or others middleware frameworks.


#7

I for some reason have an aversion to Django, I always go for the underdog (Foundation over bootstrap, Vue over React, APIStar over Django-Rest-Framework) so from my perspective they are not trying to recreate Django here and I kind of like that. The operative quote I am pinning this on is “API Star provides something very similar to middleware”

I think I usually lurk here to solve APIStar puzzles. So with what you just showed me above I say we are both right in a way. I still see an issue in App.__call__ because of how it assembles your funcs, not with get_event_hooks. Notice that the start and finalize responses are part of your funcs list. They need dependency injection too, so you are right that your issue could be with the dependency Injector, but with it’s run interface.

You would need to go to some great lengths though to get the behavior you are looking for. You would need to subclass your own Injector and App. Your app’s call function will have to call an injector that accepts the event hooks as separate lists, the handler, render_response, finalize_asgi/wsgi all separately so that you can appropriately assemble them in an order to render the response back in the event that one of your hooks returns. I don’t think that is very clean or maintainable though. You would be pushing the responsibility of ordering into the Injector, and that just doesn’t seem like it’s job.


#8

I’m not a fan of Django, in fact I’m more of a hater, I only pointed that this implementation is extremely limited. You say that you still have to craft the other components, but the reality is if a hook returns a Response and you give it as good, in the current request you don’t have to execute the rest of them, just return write this response out.

Look to another possibility, to middleware-free oldie tornado web, the option there is to define a prepare method called always before the actual handlers and a method finish that finishes the request and write a response (crafted in prepare). This could be an special component executed before and out of the rest or it can even be a normal function passed as argument to App, executed before all the chain of injected funcs.

I really would like to see a framework that does not enforce me to define an OPTIONS handler for each endpoint because it cannot handle properly flow control and otherwise raises MethodNotAllowed, and this is just a bleeding example, there are others.

Think of more serious use cases, a security hook, where you check x-headers for https or other important headers, and you want to cut the cord, but the reality is that if you raise an Exception it continues to run all on_response passing the exception, which can be dangerous, as you don’t want to follow up anymore and you don’t want all the components and all the hooks to be aware of these things.

This is why you encapsulate in a single hook and you put its on_request at the top of the list to be the first in execute and therefore to be the first to stop everything at this point, same as reverse-order on response phase, guess what, exactly the same behaviour as Django middleware.


#9

My response was stuck in the bounds of the current source implementation. I agree what we have is really limited. Personally, I have beef with the “always global” hooks. For example I don’t need a database session for all my end points, so when Components stopped supporting context managers in 0.4 I was stuck with the sub optimal hooks firing off on all my routes even though I didn’t need them triggered.

I’ve never used Twisted myself. I totally agree, writing OPTIONS for all the endpoints sucks. I thought Flask did something to give you OPTIONS right out the gate, but it has been a long time since I’ve seen that. You make a better case for this behavior with the security example.

Did you fork the repo? I’d like to see what your implementation looks like.


#10

I’ll give it a try when I have some time, for me the most obvious is to change this small piece of code once I understand completely the Injector intricacies, instead of overriding state checking on its type and returning from the inner loop, no need to loop’em all:


#11

And you are right @androiddrew, for the sake of OPTIONS and CORS it’s better to do this kind of termination in a proxy, a simple local Nginx through socket would do the trick no need to have it in ApiStar just for this reason if it introduces too much complexity, simplicity of hooks aren’t a bad thing per se.


#12

I expected that the same component instances injected on_response would be injected on_error. As they aren’t the same, instance mutations that happens before the exception context is lost, and also components must be resolved and instantiated again. So maybe the state could be preserved between hooks when possible.

I really would like to see a framework that does not enforce me to define an OPTIONS handler for each endpoint because it cannot handle properly flow control and otherwise raises MethodNotAllowed, and this is just a bleeding example, there are others.

I totally agree because I could only address this using a middleware outside of the framework

Why event hooks are all executed always? This makes impossible to make some basic hooks like CORS properly, in Django middleware if a middleware returns a response it stops there. Now the only way to cut the chain I think is by raising an exception, which is fine for some use cases, but not for others that are not errors.

The Scrapy framework changes control flow by raising exceptions, so maybe a raise EagerResponse(response) could do it (assuming that EagerResponse derives from BaseException). Also an EagerResponseThatDontTriggerOnResponseHooks would be nice. Unfortunately in Python 3 we can only raise exceptions so raise Response will not work. Otherwise return an usual Response would be also very nice, but on_response and on_error also must need to know if it was returned eagerly or not.


#13

I’d suggest something along these lines…

from apistar.exceptions import HTTPException

class Success(HTTPException):
    default_status_code = 200
    default_detail = None

Then use raise Success(detail=...).

Alternatively use standard WSGI (or ASGI) middleware. Flask does just fine without a framework-specific middleware API.


#14

This answer is more bearable, I tried to make the response thing and it works but as it happens after the router matching path and method it does not solve my issue of capturing method before it’s resolved, no way to skip MethodNotAllowed, so I won’t PR as this solution of faked exception is more concise and the things for treating OPTIONS or other unhandled methods might be easily handled with an nginx in front of the app server.